Select menus with optgroups and disabled items put GrayText on the wrong items

RESOLVED FIXED in Firefox 54

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 ?, firefox54+ fixed, firefox55 fixed)

Details

(Whiteboard: [uplift note: this patch must be applied after the patches from bug 1347089 and 1346440])

Attachments

(3 attachments)

STR:
Open data:text/html,<h2 id="bigten">Big Ten Teams</h2><p>14 items</p><select><optgroup label="East Division"><option disabled="">Michigan</option><option>Penn State</option><option disabled="">Ohio State</option><option>Maryland</option><option>Indiana</option><option>Michigan State</option><option>Rutgers</option></optgroup><optgroup label="West Division" disabled=""><option>Nebraska</option><option>Iowa</option><option>Northwestern</option><option>Minnesota</option><option>Wisconsin</option><option>Purdue</option><option>Illinois</option></optgroup></select></div>

Expected:
Only "Michigan" and "Ohio State" should be disabled in the first optgroup, and the second optgroup should have all items disabled.

Actual:
The wrong items have GrayText applied.

This is a bug because we weren't passing the correct nthChildIndex to populateChildren, and were applying the styles to potentially the wrong items also because we were switching between menucaption and menuitem but the nth-child index wasn't relative to the tagName.
[Tracking Requested - why for this release]: Visible UI regression from bug 910022.
status-firefox53: --- → ?
status-firefox54: --- → affected
status-firefox55: --- → affected
tracking-firefox54: --- → ?
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Tracking as ui regression in 54.
tracking-firefox54: ? → +
Blocks: 1347145
Attachment #8847323 - Flags: review?(mconley) → review?(dtownsend)
Attachment #8847330 - Flags: review?(mconley) → review?(dtownsend)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8847330 [details]
Bug 1347329 - Move the select tests to a forms subdirectory and split apart the color tests to their own file since the test was getting very long.

https://reviewboard.mozilla.org/r/120332/#review122644
Attachment #8847330 - Flags: review?(dtownsend) → review+

Comment 8

2 years ago
mozreview-review
Comment on attachment 8847323 [details]
Bug 1347329 - Make sure that the nth-child index gets updated when recursing so that colors get applied correctly when optgroups are used.

https://reviewboard.mozilla.org/r/120322/#review122650
Attachment #8847323 - Flags: review?(dtownsend) → review+

Comment 9

2 years ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/899f6ba2fe55
Make sure that the nth-child index gets updated when recursing so that colors get applied correctly when optgroups are used. r=mossop
https://hg.mozilla.org/integration/autoland/rev/a84419a2e52d
Move the select tests to a forms subdirectory and split apart the color tests to their own file since the test was getting very long. r=mossop
Backed out for failing browser_selectpopup_colors.js:

https://hg.mozilla.org/integration/autoland/rev/2ef7b06c606b0334e2a9634ae02b98339ffa7aeb
https://hg.mozilla.org/integration/autoland/rev/27e03731726b5baf19484e7a2c428b19f4dbff3f

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a84419a2e52da08aba308a119d68e6fee24281e1&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=84125497&repo=autoland

[task 2017-03-15T22:08:10.093107Z] 22:08:10     INFO - TEST-START | browser/base/content/test/forms/browser_selectpopup_colors.js
[task 2017-03-15T22:08:55.176050Z] 22:08:55     INFO - TEST-INFO | started process screentopng
[task 2017-03-15T22:08:56.556243Z] 22:08:56     INFO - TEST-INFO | screentopng: exit 0
[task 2017-03-15T22:08:56.561877Z] 22:08:56     INFO - Buffered messages logged at 22:08:10
[task 2017-03-15T22:08:56.562203Z] 22:08:56     INFO - Entering test bound test_colors_applied_to_popup_items
[task 2017-03-15T22:08:56.564835Z] 22:08:56     INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,%3Chtml%3E%3Chead%3E%3Cstyle%3E%20%20.blue%20%7B%20color%3A%20%23fff%3B%20background-color%3A%20%2300f%3B%20%7D%20%20.green%20%7B%20color%3A%20%23800080%3B%20background-color%3A%20green%3B%20%7D%20%20.defaultColor%20%7B%20color%3A%20-moz-ComboboxText%3B%20%7D%20%20.defaultBackground%20%7B%20background-color%3A%20-moz-Combobox%3B%20%7D%3C/style%3E%3Cbody%3E%3Cselect%20id%3D%27one%27%3E%20%20%3Coption%20value%3D%22One%22%20style%3D%22color%3A%20%23fff%3B%20background-color%3A%20%23f00%3B%22%3E%" line: 0}]
[task 2017-03-15T22:08:56.565145Z] 22:08:56     INFO - Buffered messages finished
[task 2017-03-15T22:08:56.566536Z] 22:08:56     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/forms/browser_selectpopup_colors.js | Test timed out -
Flags: needinfo?(jaws)
Comment hidden (mozreview-request)

Comment 12

2 years ago
mozreview-review
Comment on attachment 8847853 [details]
Bug 1347329 - After splitting browser_selectpopup.js up, disable it on Linux per bug 1329991.

https://reviewboard.mozilla.org/r/120774/#review122730
Attachment #8847853 - Flags: review?(dtownsend) → review+

Comment 13

2 years ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5e5baf32aa7a
Make sure that the nth-child index gets updated when recursing so that colors get applied correctly when optgroups are used. r=mossop
https://hg.mozilla.org/integration/autoland/rev/dde143cdbfc0
Move the select tests to a forms subdirectory and split apart the color tests to their own file since the test was getting very long. r=mossop
https://hg.mozilla.org/integration/autoland/rev/b22e8345c162
After splitting browser_selectpopup.js up, disable it on Linux per bug 1329991. r=mossop

Comment 14

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5e5baf32aa7a
https://hg.mozilla.org/mozilla-central/rev/dde143cdbfc0
https://hg.mozilla.org/mozilla-central/rev/b22e8345c162
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Duplicate of this bug: 1347145
Please request Aurora uplift on these patches when you get a chance.
Comment on attachment 8847323 [details]
Bug 1347329 - Make sure that the nth-child index gets updated when recursing so that colors get applied correctly when optgroups are used.

Approval Request Comment
[Feature/Bug causing the regression]: regression from bug 910022
[User impact if declined]: select menus that use optgroups can have wrong coloring
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: covered by automated tests and it has been on nightly for a few days now
[String changes made/needed]: none
Flags: needinfo?(jaws)
Attachment #8847323 - Flags: approval-mozilla-aurora?
Whiteboard: [uplift note: this patch must be applied after the patches from bug 1347089 and 1346440]
Comment on attachment 8847330 [details]
Bug 1347329 - Move the select tests to a forms subdirectory and split apart the color tests to their own file since the test was getting very long.

See comment 17. Note that these patches must be applied on top of the patches from bug bug 1347089 and 1346440.
Attachment #8847330 - Flags: approval-mozilla-aurora?
Comment on attachment 8847853 [details]
Bug 1347329 - After splitting browser_selectpopup.js up, disable it on Linux per bug 1329991.

See comment 17. Note that these patches must be applied on top of the patches from bug bug 1347089 and 1346440.
Attachment #8847853 - Flags: approval-mozilla-aurora?
Comment on attachment 8847323 [details]
Bug 1347329 - Make sure that the nth-child index gets updated when recursing so that colors get applied correctly when optgroups are used.

Fix an UI regression related to select menu. Aurora54+.
Attachment #8847323 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

2 years ago
Attachment #8847330 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

2 years ago
Attachment #8847853 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.