585799 - Deal with what are currently popup widgets (e.g. for <select>) in widgetless content processes

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

After bug 582057, navigating to a page that creates a <select> causes the content process to abort on NYI code.  We need to figure out how to deal with popups.

I think we have two approaches, roughly speaking.  One is to teach PuppetWidgets about popup widgets, and create popup PuppetWidgets.  This would entail sending the content process's "widget" hierarchy to the chrome process, because chrome would need to know the z-order of fake widgets.

The second is to add a second, non-widget implementation of the <select> frame (or something to that effect).  Then the <select> can have its own layer (or whatever), and we can reuse all the existing cross-process layer machinery and stuff will Just Work.

I personally lean towards the second option.  I'm afraid of PuppetWidgets turning into a full-fledged widget toolkit, rather than being simple stand-ins as they are now.  This also seems to me to fit into the general move towards tab-local UI, in the same spirit as tab-scoped dialog boxes.  However, I have no idea what code would break if <select>s didn't have widgets.  I also don't know what UX traditions govern <select>. 

The other factor is what the fennec frontend wants.  It appears to me that the current handling of <select>s is some relatively straightforward CSS, so that should work fine with either approach.

Thoughts?

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

In fennec, we stop the normal creation of a popup for the <select> and instead use our own, touch-friendly, UI to handle manipulating the selected item in a <select>.

This touch-friendly UI is created in the browser chrome, in the parent process. We have a fair amount of code that lives in the child process to send over the <option>s and other data needed to support the touch-friendly UI.

As long as we still have some way to skip creating a popup for the <select>, fennec should be fine. If the new mechanism was designed to support alternate UIs for the <select>, even better.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The reason we create a popup widget for <select>s is that we want a large select to be able to extend outside the browser window on desktop Firefox. We probably don't want to regress that.

Obviously for mobile this is not an issue. So for now, I suggest focusing on a solution that integrates cleanly with Fennec's select handling.

Beyond that, we probably should create a higher-level API that lets us send the options and optgroups over to the chrome process and have it implement the select functionality itself with the real window toolkit. Maybe we can use the Fennec API everywhere.

Note that we also have XUL menus doing similar things, although if we disable XUL menus in content processes it's probably not too bad.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

OK, cool.  I'll let this bug lie for the time being.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Mark and I found out this evening that we still create popup widgets when the "ui.use_native_popup_windows" pref is flipped to false.  So we need something, probably a band-aid, in place for b1.

Comment 5

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

Attachment: WIP patch, use native widget in content process

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #4)
> Mark and I found out this evening that we still create popup widgets when the
> "ui.use_native_popup_windows" pref is flipped to false.

That's not the default value, is it? Why bother supporting this pref at all in E10S Fennec?

Comment 7

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

Comment on attachment 471605 [details] [diff] [review]
WIP patch, use native widget in content process

This works pretty well for fennec as far as I can tell, changing my feedback request to a review request.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

How about comment #6? I'm wondering why we need to support ui.use_native_popup_windows=false in E10S Fennec.

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

That pref doesn't mean what I thought it did.  AFAICT it's irrelevant to fennec.  It's a red herring, sorry to sidetrack this bug.

Comment 10

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

(In reply to comment #8)
> How about comment #6? I'm wondering why we need to support
> ui.use_native_popup_windows=false in E10S Fennec.

As I understand it, the speculation was that if the ui.use_native_popup_window pref was set to false then we'd avoid this code path entirely. That's not the case, so we need to fix this bug.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK then, how does this patch work? How can we allocate a child popup widget if we don't have a real widget to make it a child of?

And why do we need to create a popup widget in the first place? Given comment #1, I thought Fennec never needed to do this.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(In reply to comment #11)
> OK then, how does this patch work? How can we allocate a child popup widget if
> we don't have a real widget to make it a child of?
> 
> And why do we need to create a popup widget in the first place? Given comment
> #1, I thought Fennec never needed to do this.

Fennec never needs to *show* a popup widget.  (AFAIK.)  For fennec, this bug encompasses the easiest, least-hacky way to keep existing <select>-related code intact in content processes, which don't have parent native widgets.

Re-reading comment 0, I think we have three options now
 (1) create native popup widgets but never use them: Brad's patch
 (2) create a hidden, not-parented PuppetWidget for popups, assert if it's ever shown
 (3) refactor the <select> code to not use popup widgets if they're not available, and instead do "something else"

Feedback appreciated :).  (1) and (2) are approximately equally easy, but pull in many unknown factors.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #12)
>  (3) refactor the <select> code to not use popup widgets if they're not
> available, and instead do "something else"

We already have the "something else" code, it seems. It seems to me that hacking nsComboboxControlFrame/nsListControlFrame to not create a widget in a content process should be straightforward.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

cjones: if we got a similar band-aid going with unparented puppet widgets, would that suit?
roc: yes!

Comment 15

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

Attachment: patch

Comment 16

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

Attachment: patch

Comment 17

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

merged to m-c from cedar http://hg.mozilla.org/mozilla-central/rev/a796f404ede5