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

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Valentin C.]

Attachment: 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>

Comment 1

[Loic]

Broken with e10s enabled.

Comment 2

[Loic]

Surely dupe of bug 910022.

Comment 3

[Valentin C.]

Yes I can confirm it solves my issue !

Comment 4

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-1242450-Do-not-display-hidden-option-elements-in.patch

Comment 5

[Mike Conley (:mconley) (:⚙️)]

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/.

Comment 6

[George Wright (:gw280) (needinfo me!)]

Ugh, can't believe I wrote that. Clearly I'm not thinking straight. I will prep a new patch.

Comment 7

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-1242450-Do-not-display-hidden-option-elements-in.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.

Comment 8

[Mike Conley (: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!

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f9e929f43a2

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6f9e929f43a2

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6f9e929f43a2

Comment 12

[George Wright (:gw280) (needinfo me!)]

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

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

It would be good to verify this on aurora once it lands there.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/46e9cc306110

Comment 16

[Valentin C.]

I still have the issue in 46.0a2 (2016-02-01) ; I don't know if it's normal...

Comment 17

[Loic]

Probably in the next Aurora.

Comment 18

[Valentin C.]

Ok, I'll update this when I can confirm it is resolved

Comment 19

[Valentin C.]

I can confirm it working :-) Thx !

Comment 20

[Valentin C.]

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.

Comment 21

[Jim Mathies [:jimm]]

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.

Comment 22

[Mike Conley (:mconley) (:⚙️)]

Yeah, what comment 20 describes is a dupe of bug 1255841. Closing this one out. Interested parties can follow along in the other bug.

Comment 23

[Cornel Ionce [:noni] [Hubs QA]]

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.

Comment 24

[Mike Conley (:mconley) (:⚙️)]

*sigh*. Sorry about that. Was pretty sure that was going to work. I'll do a full try push before I try landing again.