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

RESOLVED WONTFIX

Status

()

Core
DOM: Content Processes
RESOLVED WONTFIX
3 years ago
a year ago

People

(Reporter: KWierso, Assigned: mconley)

Tracking

(Blocks: 1 bug, {dev-doc-complete, dogfood, site-compat})

Trunk
dev-doc-complete, dogfood, site-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm9+)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

3 years ago
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

3 years ago
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
Blocks: 516752
Component: TreeStatus → General
Product: Tree Management → Firefox
Summary: Changing a tree's status is weird when run in an e10s window. → [e10s] treestatus.mozilla.org UI doesn't update correctly when run in an e10s window
Version: --- → Trunk

Updated

3 years ago
tracking-e10s: --- → ?
Keywords: dogfood

Updated

3 years ago
Summary: [e10s] treestatus.mozilla.org UI doesn't update correctly when run in an e10s window → [e10s] treestatus.mozilla.org JS form validation breaks when run in an e10s window
Assignee: nobody → wmccloskey
tracking-e10s: ? → m4+
(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??
Flags: needinfo?(emorley)
(Reporter)

Comment 3

3 years ago
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

3 years ago
(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.
Flags: needinfo?(emorley)
(Assignee)

Updated

3 years ago
Assignee: wmccloskey → mconley
(Assignee)

Updated

3 years ago
Flags: needinfo?(mconley)
(Assignee)

Comment 5

3 years ago
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.
Flags: needinfo?(mconley)
(Assignee)

Comment 6

3 years ago
Created attachment 8523959 [details]
Manual test case
(Assignee)

Comment 7

3 years ago
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.
Component: General → DOM: Content Processes
Product: Firefox → Core
Summary: [e10s] treestatus.mozilla.org JS form validation breaks when run in an e10s window → [e10s] <option> clicks do not bubble up through parent <select>
(Assignee)

Comment 8

3 years ago
This seems to apply to all intrinsic events (click, mouseout, mousedown, keyup, etc).
Summary: [e10s] <option> clicks do not bubble up through parent <select> → [e10s] <option> events do not bubble up through parent <select>

Comment 9

3 years ago
Thank you for digging into this :-)
(Assignee)

Updated

3 years ago
Depends on: 1053981
(Assignee)

Updated

3 years ago
OS: Windows 8.1 → All
Hardware: x86_64 → All
(Assignee)

Comment 10

3 years ago
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. :/
Flags: needinfo?(enndeakin)
Flags: needinfo?(ally)
I don't know much about the select element code, so I don't I can give much of a helpful answer. Sorry.
Flags: needinfo?(enndeakin)
(Assignee)

Comment 12

3 years ago
How about the other Neil then? :)
Flags: needinfo?(neil)
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.)
Flags: needinfo?(neil)
(Assignee)

Comment 14

3 years ago
(In reply to neil@parkwaycc.co.uk 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.
Flags: needinfo?(bugs)
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
So how do we currently implement <select> (combobox) in E10s?
Flags: needinfo?(bugs)
Given the plan we hatched at the pdx workweek, is there still anything you need from me?
Flags: needinfo?(ally)
(Assignee)

Comment 17

3 years ago
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."
(Assignee)

Comment 18

3 years ago
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.
tracking-e10s: m4+ → ?
tracking-e10s: ? → m7+

Updated

2 years ago
Blocks: 1154677

Comment 19

2 years ago
bumping to m8, we consider this a release blocker.
tracking-e10s: m7+ → m8+
Assignee: mconley → gwright
(Assignee)

Comment 20

2 years ago
Stealing because gw280 is on PTO.
Assignee: gwright → mconley
(Assignee)

Comment 21

2 years ago
Created attachment 8651887 [details]
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=?
(Assignee)

Comment 22

2 years ago
Created attachment 8651888 [details]
MozReview Request: Bug 1090602 - Part 2, experimentation...

Bug 1090602 - Part 2, experimentation...
(Assignee)

Comment 23

2 years ago
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.
(Assignee)

Comment 24

2 years ago
Created attachment 8674443 [details]
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.
Attachment #8523959 - Attachment is obsolete: true
(Assignee)

Comment 25

2 years ago
Created attachment 8674449 [details]
Manual test case

Some more refinements to the test case.
Attachment #8674443 - Attachment is obsolete: true
(Assignee)

Comment 26

2 years ago
Created attachment 8674454 [details]
Manual test case

Last bit of refinement.
Attachment #8674449 - Attachment is obsolete: true
(Assignee)

Comment 27

2 years ago
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
(Assignee)

Comment 28

2 years ago
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
(Assignee)

Comment 29

2 years ago
Created attachment 8674974 [details]
Manual test case

Added click event on Sunday, and defaulting to inline <option>s.
Attachment #8674454 - Attachment is obsolete: true
(Assignee)

Comment 30

2 years ago
Created attachment 8674977 [details]
Manual test case

Bah, fixup.
Attachment #8674974 - Attachment is obsolete: true
(Assignee)

Comment 31

2 years ago
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
(Assignee)

Comment 32

2 years ago
Also cross-filed to the W3C: https://github.com/w3c/uievents/issues/55
(Assignee)

Comment 33

2 years ago
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.
tracking-e10s: m8+ → ?
tracking-e10s: ? → +

Comment 34

2 years ago
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.
(Assignee)

Updated

2 years ago
Flags: needinfo?(mconley)
(Assignee)

Comment 35

2 years ago
(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.
Flags: needinfo?(mconley)
Chrome uses a native menu widget on Mac. (or something emulating it)
(Assignee)

Comment 37

2 years ago
(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

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

Comment 39

2 years ago
Alternatively, we could patch non-e10s to behave the same way as e10s. It's hard to know what the right move is here.
Flags: needinfo?(jgriffiths)
Mike, what are the web compat implications of this?
tracking-e10s: + → m9+
Flags: needinfo?(miket)
(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.)
Flags: needinfo?(miket) → needinfo?(blassey.bugs)
Keywords: site-compat
Keywords: dev-doc-needed
Flags: needinfo?(blassey.bugs)
(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 :)
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(jgriffiths)
Resolution: --- → WONTFIX
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/
Thank you, Kohei!
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!
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.