Closed Bug 1347329 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox53 --- ?
firefox54 + fixed
firefox55 --- fixed

People

(Reporter: jaws, Assigned: jaws)

References

Details

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

Attachments

(3 files)

Bug 1347329 - Make sure that the nth-child index gets updated when recursing so that colors get applied correctly when optgroups are used.
59 bytes, text/x-review-board-request
mossop
: review+
gchang
: approval-mozilla-aurora+
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.
59 bytes, text/x-review-board-request
mossop
: review+
gchang
: approval-mozilla-aurora+
Details
Bug 1347329 - After splitting browser_selectpopup.js up, disable it on Linux per bug 1329991.
59 bytes, text/x-review-board-request
mossop
: review+
gchang
: approval-mozilla-aurora+
Details 
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: --- → ?
Tracking as ui regression in 54.
tracking-firefox54: ? → +
Attachment #8847323 - Flags: review?(mconley) → review?(dtownsend)
Attachment #8847330 - Flags: review?(mconley) → review?(dtownsend)
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 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+
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 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+
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
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+
Attachment #8847330 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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.