Valentin C. Reporter
	Description • 8 years ago

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>

	Loic
	Comment 1 • 8 years ago

Broken with e10s enabled.

	Loic
	Comment 2 • 8 years ago

Surely dupe of bug 910022.

	Valentin C. Reporter
	Comment 3 • 8 years ago

Yes I can confirm it solves my issue !

	Mike Conley (:mconley) (:⚙️)
	Comment 5 • 8 years ago

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

	George Wright (:gw280) (needinfo me!) Assignee
	Comment 6 • 8 years ago

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

	George Wright (:gw280) (needinfo me!) Assignee
	Comment 7 • 8 years ago

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.

	Mike Conley (:mconley) (:⚙️)
	Comment 8 • 8 years ago

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!

	Pulsebot
	Comment 9 • 8 years ago

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

	Carsten Book [:Tomcat]
	Comment 10 • 8 years ago
bugherder

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

	Carsten Book [:Tomcat]
	Comment 11 • 8 years ago
bugherder

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

	George Wright (:gw280) (needinfo me!) Assignee
	Comment 12 • 8 years ago

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

	Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)
	Comment 13 • 8 years ago

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

	Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)
	Comment 14 • 8 years ago

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.

	Carsten Book [:Tomcat]
	Comment 15 • 8 years ago
bugherder uplift

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

	Valentin C. Reporter
	Comment 16 • 8 years ago

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

	Loic
	Comment 17 • 8 years ago

Probably in the next Aurora.

	Valentin C. Reporter
	Comment 18 • 8 years ago

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

	Valentin C. Reporter
	Comment 19 • 8 years ago

I can confirm it working :-) Thx !

	Valentin C. Reporter
	Comment 20 • 8 years ago

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.

	Jim Mathies [:jimm]
	Comment 21 • 8 years ago

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.

	Mike Conley (:mconley) (:⚙️)
	Comment 22 • 8 years ago

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

	Cornel Ionce [:noni] [Hubs QA]
	Comment 23 • 8 years ago

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.