1305017 - [e10s] Can not open the <select> drop-down list

Status: RESOLVED WORKSFORME

(Core :: Layout: Form Controls, defect, P3)

Description

[blinky]

Steps to reproduce:

1. Go to https://dl.dropboxusercontent.com/u/95157096/85f61cf7/hl2noccq1w.html.
2. Open the <select> element.


Actual results:

Can not open the <select> drop-down list.

Comment 1

[blinky]

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ac43dab284afd189ed2b0f4c1906a45e0c1736dd&tochange=9424f63828ebaf3563cad850ce1cc5f865231f70

Comment 2

[Neil Deakin]

So it looks like non-e10s selects save and restore the dropdown state when the <select> gets restyled. Right now in e10s the frame is being destroyed so the message gets sent to the parent process to close the dropdown.

Comment 3

[Jim Mathies [:jimm]]

Neil, planning on looking at this?

Comment 4

[Neil Deakin]

I'm not currently planning to look at this.

Comment 5

[Neil Deakin]

The issue here is a message is sent to the popup when the nsListControlFrame frame gets destroyed, yet a restyle in non-e10s destroys the frame, recreates it then reopens the popup if it was already open.

We need to be able to distinguish between a select that it being destroyed due to a restyle versus one that is really going away.

Note that in non-e10s you can removeChild a <select> while it is open, wait several minutes, then appendChild it again and the popup will reappear.

Maybe Mats has some idea here?

Comment 6

[Mats Palmgren (inactive)]

I'm not familiar with e10s popups at all, so I have no idea how to solve this.

I think out-of-process popups is an insane idea to begin with. ;-)

Comment 7

[Mats Palmgren (inactive)]

I think the code that makes this work in the non-e10s case are these methods:
http://searchfox.org/mozilla-central/rev/d96317a351af8aa78ab9847e7feed964bbaac7d7/layout/forms/nsComboboxControlFrame.cpp#1664
(in case that helps...)

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

we are a week away from RC, too late, this is now a wontfix for 50.

Comment 9

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

Attachment: Test case

Comment 10

[Andrew Overholt [:overholt]]

I created a screencast: https://www.dropbox.com/s/9dgqjh72oscjvce/selectitemse10s.mp4?dl=0 . It doesn't show the popup with the selectable items but it mostly illustrates the problem here.

Comment 11

[Andrew Overholt [:overholt]]

The test case uses https://github.com/harvesthq/chosen (and thus not every single <select> is affected) but maybe other frameworks could hit this. I've seen this broken on Windows, Linux, and Mac.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

This is actually less-broken for me on Aurora & Nightly these days. I still get the initial closing on the first click, but subsequent ones seem to work. Still glitchy, but not as unusable as it used to be. Mozregression says something in dholbert's push below somehow made this better, though I'm a bit at a loss as to how.

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b48820f4c87aeaa2186c401c1bafe6661d6b7e31&tochange=995211b23a553e1518e8c79570d27f5c4fd0bb86

Comment 13

[Daniel Holbert [:dholbert]]

So in current mozilla-central, the first click on <select> triggers a frame-reconstruction of that subtree's whole containing block, due to the characteristics of the inserted content.  And subsequent clicks do not. That's why it only breaks the first time.

Specifically:
 - The content is inserted into a nsInlineFrame (an inline-level frame), for the <vis-agg-param-editor> node.
 - The inserted content *is not all inline-level* (so it's going to trigger a block-in-inline split, a.k.a. "{ib}" split).
 - The first time, this {ib} split is a *new* {ib} split. So we reconstruct frames, to be on the safe side, because {ib} splits are complex. (Code-wise, we enter the !IsFramePartOfIBSplit() clause in WipeContainingBlock, and "break" down to the frame-reconstruction code at the bottom of that function.)
 - BUT on *subsequent* clicks, there's already an {ib} split, so we append to that existing split. (Specifically: we jump past the aforementioned !IsFramePartOfIBSplit() check, and then we enter the subsequent IsSafeToAppendToIBSplitInline() call, which returns false, skipping the reconstruction codepath.)

(I suspect the reason this got better recently (per comment 12) is that there's probably some abspos element inside of a flex container, whose placeholder would previously be wrapped in an anonymous flex item.  And when content is inserted inside/alongside-of an anonymous flex item, we often throw up our hands and do frame reconstruction, since it may need to be split/grown/etc.  And as of "Bug 1269045 part 3", we won't wrap that placeholder frame in an anonymous flex item anymore, so we don't have that extra reason for frame reconstruction here. Hooray, less frame reconstruction!)

If we have a contact with the affected site, we could suggest that they can work around the first-select-click issue by making the content-insertion node a *block-level* element, instead of an inline-level element, so that they're not triggering messy block-in-inline splits. (This might give them some small performance benefits from that reduced complexity, too.)

Specifically, they can fix (in Nightly 53 at least) this with the following one line of CSS:
  vis-agg-param-editor { display:block }

On our end, as noted in comment 5: it sounds like in non-e10s, we do preserve the popup even through a frame-reconstruction, so this "just works" -- but we don't have that ability with e10s popups right now. If this turns out to be a common/particularly-painful usecase, then I suppose we can add a mechanism for that. But for now, in this particular case, it seems like this has at least gotten a bit better recently & we do have a suggested workaround that the site could use if they choose to do so.

Comment 14

[Daniel Holbert [:dholbert]]

Attachment: Testcase, with suggested fix applied (adding display:block on vis-agg-param-editor)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

blinky, is this your site or someone else's? Would be nice if we could relay the info from this bug to whomever this is affecting in the wild.

Comment 16

[blinky]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #15)
> blinky, is this your site or someone else's? Would be nice if we could relay
> the info from this bug to whomever this is affecting in the wild.

The test case here is from bug 1097482 attachment index.html.

This bug will be fixed by bug 1311279.

Comment 17

[Neil Deakin]

> This bug will be fixed by bug 1311279.

It does not.

Comment 18

[Daniel Holbert [:dholbert]]

(In reply to blinky from comment #16)
> The test case here is from bug 1097482 attachment index.html.

OK, so this was from Kibana 4, per the title of that bug.  I've tried to reproduce this on a live site that I found running Kibana 4, but I could not, because the live site behaves a bit differently.

My STR were:
 1. Visit http://parlement.letemps.ch/#/visualize/step/2?type=pie&_g=()
 2. Click "From a new search" and then "split slices".
     (That gives a rendering that looks at lot like the testcase here.)
 3. Click the "Aggregation" dropdown.

* In this bug's attached testcase, the dropdown appears, which immediately triggers the insertion of more elements, which triggers frame-reconstruction, which (with e10s) makes the dropdown-menu spontaneously disappear. That's the bug.
* On the live parlement.letemps.ch site, the dropdown menu appears, but the rest of the page doesn't change until you pick an entry from the dropdown menu. So, all is well. No bug.

I tested the latest version of Kibana (Kibana 5) here, too:
http://demo.elastic.co/beats/app/kibana#/visualize/create?type=pie&savedSearchId=Apache-HTTPD&_g=()&_a=(filters:!(),linked:!t,query:(query_string:(query:%27*%27)),uiState:(),vis:(aggs:!((enabled:!t,id:%271%27,params:(),schema:metric,type:count)),listeners:(),params:(addLegend:!t,addTooltip:!t,isDonut:!f,legendPosition:right,shareYAxis:!t),title:%27New%20Visualization%27,type:pie))

...and I'm seeing the same good-behavior there as well -- the page doesn't get modified until I pick a selection from the menu.

So, I'm guessing the issue with the testcase here is just an artifact from the testcase being reduced at some point, and it's not something that's exhibited on an actual live site anywhere. But if anyone can find this being an issue on a live site, please do note it here! (And then we could perhaps file a bug at https://github.com/elastic/kibana/issues/ to ask Kibana to make my suggested CSS fix from comment 14.)

Comment 19

[blinky]

(In reply to Neil Deakin from comment #17)
> > This bug will be fixed by bug 1311279.
> 
> It does not.

I can not reproduce this bug with the build (includes patch from bug 1311279).

https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-win32/1479394649/

Comment 20

[blinky]

(In reply to blinky from comment #19)
> I can not reproduce this bug with the build (includes patch from bug
> 1311279).
> 
> https://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-
> win32/1479394649/

The patch in bug 1311279 is fixed this issue, but another bug appeared, rectangle does not appear when open the drop-down list, only when close is appears.

See https://dl.dropboxusercontent.com/u/95157096/85f61cf7/8wslwa5lhs.mp4

Comment 21

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

Too late for a fix for 53, as we are in the last week of the 53 beta cycle.

Comment 22

[Hsin-Yi Tsai (she/her) [:hsinyi]]

[Triage] it obviously impacts user behaviour tho not a blocker as user could workaround this quite easily, e.g. click the second time, so remains P2.

Hi Mike, feel free to reassign if you are not working on it. thanks :)

Comment 23

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

Yes, I'm afraid I'm too bogged down with Photon and Quantum Flow work to address this in the near term. Unassigning.

Comment 24

[Ben Tian (inactive)]

Cannot reproduce original bug (comment 0) on Mac as comment 20.

Note on Safari neither does the rectangle appear when dropdown list is opened, but on Chrome it appears when dropdown list is opened.

Comment 25

[Ben Tian (inactive)]

(In reply to Ben Tian [:btian] from comment #24)
> Note on Safari neither does the rectangle appear when dropdown list is
> opened, but on Chrome it appears when dropdown list is opened.

The difference results from firing timing of 'onclick' event. On Mac both Firefox and Safari fire when dropdown list is closed, while Chrome fires when it's opened.

Comment 26

[Ben Tian (inactive)]

(In reply to Ben Tian [:btian] from comment #24)
> Cannot reproduce original bug (comment 0) on Mac as comment 20.

Correction: the bug remains when user opens popup at first time:(

Comment 27

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 28

[blinky]

This bug has been fixed in bug 1440506.

Pushlog for fix range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=7128cbbb244ccad8c86008ef4f2476359454f250&tochange=0ce3566b9e52520c65af601d1ff0b040e5f9ebb8