Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

select option value attribute is not set if value is equal to text #4956

Closed
hanneskuettner opened this issue Nov 17, 2021 · 1 comment · Fixed by #4959
Closed

select option value attribute is not set if value is equal to text #4956

hanneskuettner opened this issue Nov 17, 2021 · 1 comment · Fixed by #4959
Assignees
Labels
🐞 bug Something isn't working 🔩 p2-edge-case

Comments

@hanneskuettner
Copy link

Version

3.2.22

Reproduction link

sfc.vuejs.org/

Steps to reproduce

Inspect the select options and see that the option with value === textContent has no value set.

What is expected?

Regardless of the textContent I expected an option to correctly have the value attribute and not silently remove it.

What is actually happening?

The value attribute is removed.


I have verified that this is not happening in vue2. Reproduction link

If I'm missing some part of the option spec please point me to it. But as far as I am seeing it it is perfectly valid to have an option with value = textContent and I our case needed for e2e testing frameworks like Selenium to select a predefined value.

@posva posva added 🐞 bug Something isn't working 🔩 p2-edge-case labels Nov 17, 2021
@LinusBorg
Copy link
Member

LinusBorg commented Nov 17, 2021

It happens because

  1. We first set the options textContent before the value property

https://github.com/vuejs/vue-next/blob/f454dd62ab689b348902b01f849ca5347e91ffae/packages/runtime-core/src/renderer.ts#L655

  1. then, when attempting to set the value prop, we compare the new, incoming value to the existing one - and only set the property if there's a difference,.

https://github.com/vuejs/vue-next/blob/f454dd62ab689b348902b01f849ca5347e91ffae/packages/runtime-core/src/renderer.ts#L699

and this (no idea why github decided not to inline code for this one link 🤷🏻‍♂️):

https://github.com/vuejs/vue-next/blob/f454dd62ab689b348902b01f849ca5347e91ffae/packages/runtime-dom/src/modules/props.ts?_pjax=%23js-repo-pjax-container%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%2C%20%5Bdata-pjax-container%5D#L34

  1. Problem is: el.value returns the textContent when no value prop is set.

So el.value === newValue is returning true even though there's no value being set yet - but we skip setting it because of the appearant equality between "old" and "new" value

Not sure about the best solution yet - set element Text after setting the value prop isn't gonna work as we do it before explicitly so that <select value> works.

So we might need some edge case handling in patchDOMProp()'s implementation?

@LinusBorg LinusBorg self-assigned this Nov 17, 2021
LinusBorg added a commit that referenced this issue Nov 17, 2021
even when they have a matching textContent

close #4956
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Something isn't working 🔩 p2-edge-case
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants