Closed Bug 1203668 Opened 9 years ago Closed 9 years ago

changing the value of a select to a value that matches none of the options should put it in a "no option selected" state even when it's a combobox (size=1)

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla46
Tracking Flags:
Tracking Status
firefox46 --- fixed

People

(Reporter: thsilva, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(2 files, 2 obsolete files)

part 1
3.35 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
part 2 - Fix dependency on this bug in devtools code
982 bytes, patch
bzbarsky
: review+
Details | Diff | Splinter Review 
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/45.0.2454.85 Safari/537.36

Steps to reproduce:

Enter in:
http://www.w3schools.com/jsref/tryit.asp?filename=tryjsref_select_value
And change this line:
    document.getElementById("mySelect").value = "banana";
with this one:
    document.getElementById("mySelect").value = "banasdana";


Actual results:

The selected value still the first value. 


Expected results:

In:
http://www.w3schools.com/jsref/prop_select_value.asp

Specifies the value of an <option> element in a drop-down list that should get selected. If the value does not exist, the drop-down list will display an empty option

"If the value does not exist, the drop-down list will display an empty option"

I was expecting an empty option and not the first value
Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core
https://html.spec.whatwg.org/multipage/forms.html#the-select-element
"
On setting, the value attribute must set the selectedness of all the option elements
in the list of options to false, and then the first option element in the list of options,
in tree order, whose value is equal to the given new value, if any, must have its
selectedness set to true and its dirtiness set to true.

NOTE: This can result in no element having a selectedness set to true even in the case of
the select element having no multiple attribute and a display size of 1.
"

Other UAs seems to implement it correctly: Chrome, Safari, IE.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: 37 Branch → unspecified
Attached patch part 1 — — Splinter Review 
This seems to work locally.
Assignee: nobody → mats
You are fast. tks Mats
Comment on attachment 8659598 [details] [diff] [review]
part 1

The failures in the Try run looks like it's unrelated.

Source link for more context:
http://mxr.mozilla.org/mozilla-central/source/dom/html/HTMLSelectElement.cpp?rev=e9bb1fc9977a#1189
Attachment #8659598 - Flags: review?(bzbarsky)
Comment on attachment 8659598 [details] [diff] [review]
part 1

Right now I believe we have the invariant that some option is always selected in a combobox.  There's other code in HTMLSelectElement, and I'm pretty sure in combobox control frame, that depends on that invariant.

Which parts of this code have you audited and why is it ok with the invariant change?  What is the actual UI behavior (e.g. if you click the combobox to see the dropdown, or tab to it and use arrow keys to change the value) that results in this state?
Flags: needinfo?(mats)
Attachment #8659598 - Flags: review?(bzbarsky)
Summary: changing the value of a select → changing the value of a select to a value that matches none of the options should put it in a "no option selected" state even when it's a combobox (size=1)
(In reply to Boris Zbarsky [:bz] from comment #6)
> Right now I believe we have the invariant that some option is always
> selected in a combobox.

I think you're mistaken.  Does this example show that we do NOT have
such an invariant?
data:text/html,<select size=1 onfocus="this.selectedIndex=-1"><option>hello

> There's other code in HTMLSelectElement, and I'm
> pretty sure in combobox control frame, that depends on that invariant.

Can you point out the places you know of, if any?

> Which parts of this code have you audited and why is it ok with the
> invariant change?

None, because I believe we do not have such an invariant (the example
above deselects the option also in FF 3.6, so at least we haven't had
this invariant for a very long time).

> What is the actual UI behavior (e.g. if you click the
> combobox to see the dropdown,

Clicking it shows the dropdown menu as usual, with no option highlighted.
If you move the mouse over the menu it highlights the option under the
mouse as usual.  After moving the mouse outside the menu (without
selecting anything) nothing is highlighted.  Clicking outside the menu
leaves the combobox state as is, i.e. nothing is selected.

> or tab to it and use arrow keys to change the
> value) that results in this state?

Tabbing to it focus it but doesn't change the value.  Using the arrow
keys changes the value as usual, and once you do there is no way to
get back to the unselected state (for the user).  (It *should* be
possible to type ESC to undo the value change and thus unselect
all options again but I think we have an open bug on that somewhere,
possibly bug 307345).

(BTW, I'm testing this on Linux.  Hopefully other platforms behaves
reasonably similar.)
Flags: needinfo?(mats)
> Does this example show that we do NOT have such an invariant?

Yes, it does.  I'll assume then that whoever made that unselect all options already made things work correctly in that setup.
Just another information.
In my case, if you select the first option, the event onchange is not fired.
Right, that's because the first option is already selected so we do not
detect a change.  That should be fixed now, i.e. going from nothing
selected to something selected should trigger onchange.  Please verify
this in a Nightly build (once the fix shows up there, sometime next week
presumably):  https://nightly.mozilla.org/
(and please let us know if you see any other weirdness)
Flags: in-testsuite+
Attachment #8659598 - Attachment description: wip → part 1
Attached patch part3-gaia (obsolete) — — Splinter Review 
This should fix the dependency in this bug in Gaia code, which caused
the "Gu" failures.  I don't know how to submit this patch to Try for
testing though.  Any hints would be appreciated.
Attached patch part3-gaia (obsolete) — — Splinter Review 
Note: untested since I don't know to submit Gaia patches to Try.

It appears tzSelect() depends on this bug, i.e. setting a bogus value
on a <select> leaves the current value unchanged rather than the new
behavior which is to deselect all options.  I don't know what the
real fix is for this function, but this should emulate the old
<select> behavior I think.
Attachment #8660358 - Attachment is obsolete: true
Attachment #8660359 - Flags: review?(fabien)
Mats, https://wiki.mozilla.org/ReleaseEngineering/TryServer#Using_a_custom_Gaia has directions for doing a try push with a custom Gaia.  Basically, you need to fork Gaia on github, make your changes, push them to your github fork on some branch, then point try to that changeset as the Gaia to use.
Comment on attachment 8660357 [details] [diff] [review]
part 2 - Fix dependency on this bug in devtools code

r=me
Attachment #8660357 - Flags: review?(bzbarsky) → review+
It might take quite a while before I have time to spend on this bug again.

Jet, can you perhaps find someone that works on Gaia to reivew+test+land
part 3?  Once that is done we can land part 1 and 2.
Flags: needinfo?(bugs)
Tim: can you help get the "part3-gaia" patch landed per comment 21? Thanks!
Flags: needinfo?(bugs) → needinfo?(timdream)
Interesting. Why would the value not actually exist in the <select> list when we construct the list?

This should be another bug to fix (and arguably might not be a bug blocking this fix.)

I'll do that tomorrow (keeping needinfo).
As for the patch itself, to keep the old behavior, it make more sense to set selectedIndex to 0 when it's -1 instead of remembering and set the old index?

I don't think -1 and the empty value is ever expected in the rest of the file, so that's probably the safer approach too.
... but again, there is a Gu test failure.
Filed and pull request & try submitted in bug 1218677.
Flags: needinfo?(timdream)
Comment on attachment 8660359 [details] [diff] [review]
part3-gaia

(this part will land in bug 1218677)
Attachment #8660359 - Attachment is obsolete: true
Attachment #8660359 - Flags: review?(kaze)
I found a workaround for this problem. 

If you are not sure if the assigned value can be found in the listbox use jquery:

$('#listbox1').val('TestValue');

Using this syntax leads to the correct selectedIndex -1 if the assigned value doesn't match.
https://hg.mozilla.org/mozilla-central/rev/a64dd62f4c12
https://hg.mozilla.org/mozilla-central/rev/81df08547187
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Depends on: 1232853
This is going to break sites (it broke one of mine at https://mnoorenberghe.github.io/bz-dependency-buglist/) so marking site-compat. I thought only Safari had this unusual unselected behaviour but I guess it's the way all browsers are going…

In my case I assumed that if none of the options are selected then the first option would be by default which is no longer true.
Keywords: site-compat
> I thought only Safari had this unusual unselected behaviour

Per comment 1, everyone that isn't Firefox did.  So this will only break sites that already didn't work in every other browser...

> In my case I assumed that if none of the options are selected then the first option would be by default

By default, yes.  Testcase:

  data:text/html,<select><option>a<option>b

This bug was about what happens when the page then explicitly sets value to something that matches none of the options.
(In reply to Boris Zbarsky [:bz] from comment #33)
> > I thought only Safari had this unusual unselected behaviour
> 
> Per comment 1, everyone that isn't Firefox did.  So this will only break
> sites that already didn't work in every other browser...

I think this is more recent behaviour for IE (though I code be wrong) and could still break sites with different code for Gecko.

> > In my case I assumed that if none of the options are selected then the first option would be by default
> 
> By default, yes.  Testcase:
> 
>   data:text/html,<select><option>a<option>b
> 
> This bug was about what happens when the page then explicitly sets value to
> something that matches none of the options.

Yeah, sorry, I realized after commenting that this was due to how my code worked and was a step removed from this change but I decided not to comment to reduce spam on the bug.
Keywords: dev-doc-needed
This is not a recent behavior in IE. I have an application with more than 10 years that works in IE with this behavior.
Depends on: 1266078
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: