1090602 - [e10s] <option> events do not bubble up through parent <select>

Status: RESOLVED WONTFIX

(Core :: DOM: Content Processes, defect)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I think there's some weird timing issue when treestatus is running in an e10s window.

Steps:
0. Open an e10s window in Firefox Nightly.
1. Sign in to treestatus.
2. Select a (closed) tree from the list.
3. Change the 'new state' to another open and optionally clear the "reason" textbox.

Expected:
4. The Submit button changes from disabled and "No reason tags selected!" to enabled and "Submit".

Actual:
4. Submit button stays disabled and "No reason tags selected!"

The submit button only enables itself once you check one of the reason checkboxes. You can check one and uncheck it and the button will become and stay enabled.

Comment 1

[Ed Morley [:emorley]]

This bug needs to be reported against Nightly-e10s, rather than treestatus. Even if treestatus is doing something weird, other commonly used third party sites may be doing the same, and e10s cannot afford to break them.

I tried to reproduce, but using e10s made my browser crash just trying to log into treestatus.m.o

Adjusting bug using the component/CC/meta bug specified for the file-a-bug link on https://wiki.mozilla.org/Electrolysis#Contributing

The javascript in question can be found here:
https://github.com/mozilla/treestatus/blob/master/treestatus/templates/tree.html#L19

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Ed Morley [:edmorley] from comment #1)
> I tried to reproduce, but using e10s made my browser crash just trying to
> log into treestatus.m.o
Ed, can you file a bug for this crash? Preferably with steps??

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

If it's the crash I was experiencing on various sites before disabling all accessibility features, it was bug 1088148, which should now be fixed in either today's nightly or tomorrow's.

Comment 4

[Ed Morley [:emorley]]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #2)
> Ed, can you file a bug for this crash? Preferably with steps??

crash-stats.m.o had several already filed bugs, which is why I didn't file before, but have filed bug 1092216 in case it isn't a dupe.

Comment 5

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

I can reproduce this, and I suspect this is a focus event issue. I'll see if I can come up with a minimal test case.

Comment 6

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

Attachment: Manual test case

Comment 7

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

Not a focus event issue - it's a click event issue. In non-e10s, a click event bubbles up from the <option> and gets handled by the click handler on the <select> element.

In e10s, no such event bubbles up.

Comment 8

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

This seems to apply to all intrinsic events (click, mouseout, mousedown, keyup, etc).

Comment 9

[Ed Morley [:emorley]]

Thank you for digging into this :-)

Comment 10

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

So I've been looking at this bug for a few days, and here's what I've found out:

The nsListControlFrame listens to several events: keydown, keypress, mousedown, mouseup, and mousemove. If we can capture those events for the <option> elements on the parent side, send them down to the child, and replay them upon the corresponding <option> elements in the child, we get all of the battle-tested logic for the select element without too much fuss.

The problem is, however, that the nsListControlFrame, when it's in "dropdown" mode[1], manipulates and checks the state of that popup in various methods that are fired when those 5 events are dispatched to it.

So that means, in order for this plan to work, we'd need to trick nsListControlFrame into thinking that a popup is open, when in fact there is no popup open in its process. We do similar things for printing and the file input dialog, where we implement the interface on the child side, but then proxy method calls to the parent over IPC.

I guess that's what I'm proposing here, but I want a sanity check to make sure it makes sense.

ally, Enn - how feasible is it to create a new implementation of nsIComboboxControlFrame that implements some IPDL to communicate various things with the actual popup being opened on the parent side? I notice, for example, that there are some cases where the nsIComboboxControlFrame (mComboboxFrame) gets casted directly to an nsComboboxControlFrame, or QI'd to an nsIFrame, and we'd need to probably find ways around that (perhaps by adding methods to the nsIComboboxControlFrame that both the proxy and the original implementation can implement to hide the underlying implementation details).

So suppose we were to do that - what I see is something like this:

In the content process, we render a <select> element, and nsCSSFrameConstructor, seeing that it's in the content process, knows to instantiate and pass our proxy when calling SetComboboxFrame.

Once instantiated, the implementation sends the constructor message over IPC to the parent, which prepares the popup on the parent side. This would, unfortunately, have to occur in native code. The content process would need to serialize and communicate the children of the <select> up to the parent.

The parent then populates the popup, and listens for those 5 events on its cloned <option> and <optgroup> elements. On hearing them, those events are serialized and dispatched down to the child where they're replayed upon the associated <option> elements in the child. The nsListControlFrame then should, in theory, "behave normally", since our mocked our nsComboboxControlFrame is passing information to it which is consistent with the state in the parent.

This is all kinda sketchy, and based on only a few hours of me studying nsListControlFrame and friends, so it might be pretty half-baked. But I wanted to float it as an approach - especially because ally is working on bug 1053981 which is concerned with many of the same things[2].

Is there anything here I'm not considering?

[1]: The other mode is for multiple selections. In this mode, there is no popup/dropdown - we render the list in the page, and we do this successfully in content.
[2]: ally - I know I said we would likely stick with the JSMs, but if we end up needing to proxy out the nsIComboboxControlFrame into some kind of IPDL parent/child pair, it might make more sense to pack all of the logic of those JSMs into the parent/child instead. :/

Comment 11

[Neil Deakin]

I don't know much about the select element code, so I don't I can give much of a helpful answer. Sorry.

Comment 12

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

How about the other Neil then? :)

Comment 13

[[e-mail]]

If it's a problem with getting click events to fire in the right place then smaug is probably your man. (But then again I don't know what part of this is being handled in the parent - I don't know if it's possible to get the child to paint the widget and receive events directly.)

Comment 14

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

(In reply to [e-mail] from comment #13)
> If it's a problem with getting click events to fire in the right place then
> smaug is probably your man. 

Alrighty, over to smaug then. :)

> (But then again I don't know what part of this
> is being handled in the parent - I don't know if it's possible to get the
> child to paint the widget and receive events directly.)

The way I'm picturing this, the rendering of the dropdown widget will be in the parent (my understanding is that it _has_ to be done in the parent), and any of those five events[1] that occur on that dropdown in the parent need to be dispatched to... I guess the nsListControlFrame in the child. And then the child can decide on how to react to those events by reading dropdown state synchronously over IPC.

It's all kinda theoretical though - I've not studied this code that much. I'm also very open to alternatives.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

So how do we currently implement <select> (combobox) in E10s?

Comment 16

[Allison Naaktgeboren :ally]

Given the plan we hatched at the pdx workweek, is there still anything you need from me?

Comment 17

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

I don't think so - I think we're going to go with your idea basically, which was "try to special-case the content-process case in nsListControlFrame so that it doesn't have to care about the state of its dropdown to respond to events."

Comment 18

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

I've done quite a bit of digging at this, and I keep oscillating between "this won't be so bad", and "wow, this is harder than I thought". :/ I think there are likely bigger fish to fry right now, and maybe we can come back to this.

Nomming for retriage - my vote is m6 or m7.

Comment 19

[Jim Mathies [:jimm]]

bumping to m8, we consider this a release blocker.

Comment 20

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

Stealing because gw280 is on PTO.

Comment 21

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

Attachment: MozReview Request: Bug 1090602 - <option> events do not bubble up through parent <select>. r=?

Bug 1090602 - <option> events do not bubble up through parent <select>. r=?

Comment 22

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

Attachment: MozReview Request: Bug 1090602 - Part 2, experimentation...

Bug 1090602 - Part 2, experimentation...

Comment 23

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

So I've posted my WIP / experimentation code here for someone to pick up in case they want to tackle this bug while I'm gone.

If somebody does that, they might want to read my bugnotes: https://www.evernote.com/l/AbIaeqtOzshJaogk1omBE_v_Dle4IbP8a4o

Specifically, the last few paragraphs show you what the current problems are.

Comment 24

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

Attachment: Manual test case

So the good news is that the original report in comment 0 no longer applies because TreeStatus has changed to use the onchange handler on the <select> instead of onclick (I'd point to that change in the code, except that it's all written in AngularJS which I'm not familiar with... suffice it to say that the event listener on the <select> is now listening for the change event).

Here's some interesting information:

The support for firing events on <option>'s is really spotty and inconsistent across browsers.

Here's a more general test case to test across other browsers.

Comment 25

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

Attachment: Manual test case

Some more refinements to the test case.

Comment 26

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

Attachment: Manual test case

Last bit of refinement.

Comment 27

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

Here are my findings for each of the browsers I tested:

Okay! Let’s do a quick study on <option> event support across browsers!


# Firefox (non-e10s - 44.0a1 (2015-10-15)):

* On hovering <option>’s with the mouse, mouseout, mousemove, mouseover are fired for each <option>
* On clicking an <option>, mousedown, mouseup, and click are fired for the clicked <option>
* keydown and keyup fired on the <select> when keying through each <option> in the dropdown
* keydown and keyup fired when hitting Enter to select an item


# Safari (Version 9.0 (10601.1.56.2)):

* When hovering <option>’s with the mouse, no events are fired.
* On click, the <select> gets mouseout, mouseup, mouseover, click, mouseout.
* Nothing is fired when keying through each <option> in the dropdown
* When pressing Enter on a selection, the <select> gets mouseout, mouseup, mouseover, click, keyup, and then mouseout


# Chrome (Version 46.0.2490.71 (64-bit))

* When hovering <option>’s, no events are fired on either the <option>’s or the <select>
* On clicking an <option>, no events are fired on either the <option>’s or the <select>
* Nothing is fired when keying through each <option> in the dropdown
* Nothing is fired when pressing Enter on a selection


# Internet Explorer (11.0.9600.18015)

* When hovering <option>’s, no events are fired on either the <option>’s or the <select>
* On clicking an <option>, the <select> fires a mousedown and a mouseup, and then the clicked <option> fires a <click> event.
* keydown and keyup are fired on the <select> when keying through each <option> in the dropdown
* keydown, keypress and keyup are fired on the <select> when pressing Enter on a selection


# Edge (20.10240.16384.0)

* When hovering <option>’s, no events are fired on either the <option>’s or the <select>
* On clicking an <option>, the <select> sees a mousedown, mouseup and then the clicked <option> fires a <click> event.
* keydown and keyup are fired on the <select> when keying through each <option> in the dropdown
* keydown, keypress and keyup are fired on the <select> when pressing Enter on a selection

Comment 28

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

Sent email to dev-platform suggesting that we converge with Chrome's behaviour here: https://groups.google.com/forum/#!topic/mozilla.dev.platform/9Li1-qBaM88

Comment 29

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

Attachment: Manual test case

Added click event on Sunday, and defaulting to inline <option>s.

Comment 30

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

Attachment: Manual test case

Bah, fixup.

Comment 31

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

Submitted an issue to the WHATWG to hammer out a clearer spec on how (if ever) to dispatch events for <option> elements in dropdown <select>s.

https://github.com/whatwg/html/issues/263

Comment 32

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

Also cross-filed to the W3C: https://github.com/w3c/uievents/issues/55

Comment 33

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

This bug was marked M8 because we assumed it needed to be fixed in order to avoid breaking web compatibility.

I have now established via comment 27 and the issues I filed in comment 31 and comment 32 that the web compatibility requirements are undefined.

If we do nothing, we essentially match Chromium / Blink's behaviour (but break Firefox's original behaviour).

In any case, there doesn't seem to be a web compatibility risk here because of the fuzziness of the requirements. Re-requesting triage.

Comment 34

[Šime Vidas]

Hey Mike, I'm testing in Chrome 46 on Windows 10, and I get different results:

1. clicking an <option> fires "mouseup", "click", and "mouseout" events on the <select>
2. pressing Enter on a selection fires "mouseup", and "click" (!!) on the <select>; if the <select> was closed, pressing Enter opens the dropdown and fires "keydown", and "keypress" on the <select>

Also note that keying through options does fire key events if the dropdown is closed.

Comment 35

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

(In reply to Šime Vidas from comment #34)
> Hey Mike, I'm testing in Chrome 46 on Windows 10, and I get different
> results:
> 
> 1. clicking an <option> fires "mouseup", "click", and "mouseout" events on
> the <select>
> 2. pressing Enter on a selection fires "mouseup", and "click" (!!) on the
> <select>; if the <select> was closed, pressing Enter opens the dropdown and
> fires "keydown", and "keypress" on the <select>
> 
> Also note that keying through options does fire key events if the dropdown
> is closed.

Wow - yes, I'm seeing the same thing on my Windows 7 machine running Chrome! I originally did all of my testing (minus the Edge and IE testing) from comment 27 on OS X.

That's very interesting - Chrome isn't consistent across operating systems.

Comment 36

[Neil Deakin]

Chrome uses a native menu widget on Mac. (or something emulating it)

Comment 37

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

(In reply to Neil Deakin from comment #36)
> Chrome uses a native menu widget on Mac. (or something emulating it)

Right, which is probably what all browsers should be striving for - but the inconsistency in how it dispatches events is a bit surprising.

Comment 38

[Mardeg]

Right now if we don't patch e10s to match non-e10s Firefox capability, we're introducing more inconsistency (across versions) rather than less.

Comment 39

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

Alternatively, we could patch non-e10s to behave the same way as e10s. It's hard to know what the right move is here.

Comment 40

[Brad Lassey [:blassey] (use needinfo?)]

Mike, what are the web compat implications of this?

Comment 41

[Mike Taylor [:miketaylr]]

(Sorry for taking so long to reply here.)

OK, so based on the (really nice) summary Mike has given thus far, especially Comment #27, we can conclude that web browsers are generally insane.

My gut feeling is that there are likely no major compat implications beyond the current state of affairs if we don't fix this for e10s. *Some sites* may break (hence this bug report), but it seems pretty clear that if you're relying on x-browser click events to bubble up to select, you're gonna have a bad time.

We've also never supported this on Fennec, and I don't know about any mobile sites breaking -- otherwise I would have checked in the patch I wrote for Bug 715990 a few years back (we should WONTFIX that, I think, btw).

I would recommend we WONTFIX this, document the change, and teach people to listen for change events on <select>, rather than worry about <option>. That seems to be the general advice anyways for people asking similar questions:

http://stackoverflow.com/questions/1402227/click-event-on-select-option-element-in-chrome
http://stackoverflow.com/questions/7080213/jquery-click-event-not-working-on-option-element
http://stackoverflow.com/questions/5749597/jquery-select-option-click-handler

(Comment #38 is correct that we would be adding more entropy to the browser landscape by not fixing e10s to be consistent with non-e10s. Sorry about that.)

Comment 42

[Jeff Griffiths (:canuckistani) (:⚡︎)]

(In reply to Mike Taylor [:miketaylr] from comment #41)
...
> I would recommend we WONTFIX this, document the change, and teach people to
> listen for change events on <select>, rather than worry about <option>. That
> seems to be the general advice anyways for people asking similar questions:

BOOM. Done, and really appreciate your perspective :)

Comment 43

[Kohei Yoshino]

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/events-on-option-no-longer-bubble-up-to-select-in-multi-process-firefox/

Comment 44

[Mike Taylor [:miketaylr]]

Thank you, Kohei!

Comment 45

[Eric Shepherd [:sheppy]]

A number of improvements have been made to docs on MDN to cover and support this:

Cleanup and sample fixes on https://developer.mozilla.org/en-US/docs/Web/Events/change

These pages got notes mentioning the change:

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/option
https://developer.mozilla.org/en-US/docs/Web/API/HTMLSelectElement

And I've created a page for covering any e10s related changes to Web content behavior, such as this one: https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Web_content_compatibility

Please have a look and if you see any problems, feel free to comment and set the keyword back to dev-doc-needed. Thanks!