Closed Bug 1242450 Opened 4 years ago Closed 4 years ago

[e10s] Hidden attribute not working on <option> in <select> dropdown

Categories

(Core :: Layout: Form Controls, defect)

45 Branch
x86_64
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla47
Tracking Status
e10s m8+ ---
firefox46 --- verified
firefox47 --- verified

People

(Reporter: valentin, Assigned: gw280)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Attached file Test case
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160121004039

Steps to reproduce:

See test case : using <option hidden> does not work on FF45 (Linux, Gnome3), but it does on FF43 (Linux, Gnome2). Could be a regression ? Only on Linux ? (there were GTK3 updates, not so long ago ?)


Actual results:

The line with "hidden" is displayed


Expected results:

The line with "hidden" shouldn't be displayed.

See below the <select>, an example with <p>
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Broken with e10s enabled.
Blocks: e10s, e10s-select
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout: Form Controls
Ever confirmed: true
Product: Firefox → Core
Summary: Hidden attribute not working on <option> dropdown → [e10s] Hidden attribute not working on <option> in <select> dropdown
Surely dupe of bug 910022.
Yes I can confirm it solves my issue !
Assignee: nobody → gwright
Comment on attachment 8712414 [details] [diff] [review]
0001-Bug-1242450-Do-not-display-hidden-option-elements-in.patch

Review of attachment 8712414 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/modules/SelectContentHelper.jsm
@@ +109,5 @@
>        let info = {
>          tagName: tagName,
>          textContent: textContent,
>          disabled: child.disabled,
> +        hidden: child.hidden,

If it's hidden, do we even need to send it up to the parent? Might as well just skip it, I guess.

Note that script that attempts to set the hidden attribute while the popup is open won't take effect until the popup is re-opened. See https://jsfiddle.net/8nnh6sL6/.
Attachment #8712414 - Flags: review?(mconley) → review-
Ugh, can't believe I wrote that. Clearly I'm not thinking straight. I will prep a new patch.
OK, let's not do this in a really stupid way this time. Super trivial now.

As for attributes changing whilst the popup is open, that affects more than just the hidden attribute, and will require quite a bit more work. Let's fix the hidden issue here, and I'll file a followup bug for that issue.
Attachment #8712414 - Attachment is obsolete: true
Attachment #8712492 - Flags: review?(mconley)
Comment on attachment 8712492 [details] [diff] [review]
0001-Bug-1242450-Do-not-display-hidden-option-elements-in.patch

Review of attachment 8712492 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, thanks gw280. I only inspected this and didn't test it - I assume you have and that it works properly. If so, land away!
Attachment #8712492 - Flags: review?(mconley) → review+
https://hg.mozilla.org/mozilla-central/rev/6f9e929f43a2
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment on attachment 8712492 [details] [diff] [review]
0001-Bug-1242450-Do-not-display-hidden-option-elements-in.patch

Approval Request Comment
[Feature/regressing bug #]: regression from e10s, has never worked
[User impact if declined]: hidden <option> elements aren't hidden in e10s
[Describe test coverage new/current, TreeHerder]: local testing, m-c
[Risks and why]: low/no risk, very small change that only affects options with the hidden attribute.
[String/UUID change made/needed]: none
Attachment #8712492 - Flags: approval-mozilla-aurora?
It would be good to verify this on aurora once it lands there.
Flags: qe-verify+
Keywords: regression
Comment on attachment 8712492 [details] [diff] [review]
0001-Bug-1242450-Do-not-display-hidden-option-elements-in.patch

Fix for e10s related regression, please uplift to aurora.
Attachment #8712492 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I still have the issue in 46.0a2 (2016-02-01) ; I don't know if it's normal...
Probably in the next Aurora.
Ok, I'll update this when I can confirm it is resolved
I can confirm it working :-) Thx !
In fact there is still a problem : on my test case, click on "test4" or "test5" and you will see the previous item selected (respectively "test3" and "test4".

It behaves like "test3" was displayed and taking a certain height.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Is this related to bug 1255841 in some way? Since a patch landed and uplifted here lets closed this out and file a follow up for remaining and get that it into triage.
Flags: needinfo?(gwright)
Yeah, what comment 20 describes is a dupe of bug 1255841. Closing this one out. Interested parties can follow along in the other bug.
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Reproduced the initial issue on Ubuntu 14.04 x64 and Windows 10x64 using Fx 44 beta 9.

Confirming the fix on: Windows 10 x64, Mac OS X 10.9.5 and Ubuntu 14.04 using:
- latest 47.0a2 Aurora, build ID: 20160410004056;
- Fx 46.0b9, build ID:  20160407053945.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
QA Contact: cornel.ionce
You need to log in before you can comment on or make changes to this bug.