1425088 - Change the dropdown frame of <select> to a top-level absolute frame

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, enhancement, P1)

Description

[Chih-Hsuan Kuo [:tommykuo]]

In bug 1421229, we want to change the design of <select> dropdown to render the dropdown in the content process. The first step is changing the dropdown frame to a top-level absolute frame.

Comment 1

[Chih-Hsuan Kuo [:tommykuo]]

Hi mats, I think we have to do this behind a pref. Do we have any mechanism to read different styles according to a pref in the UA stylesheet? I have looked at this[1], but I don't think this is what I want.

[1]: https://searchfox.org/mozilla-central/search?q=-moz-bool-pref&case=false&regexp=false&path=

Comment 2

[Chih-Hsuan Kuo [:tommykuo]]

-moz-bool-pref works for me.

Comment 3

[Chih-Hsuan Kuo [:tommykuo]]

Attachment: Bug 1425088 - (Part 1) Add new pref to enable content-select.

Review commit: https://reviewboard.mozilla.org/r/210398/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210398/

Comment 4

[Chih-Hsuan Kuo [:tommykuo]]

Attachment: Bug 1425088 - (Part 2) Change the dropdown frame to a top-level absolute frame when content-select is enabled.

Review commit: https://reviewboard.mozilla.org/r/210400/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210400/

Comment 5

[Chih-Hsuan Kuo [:tommykuo]]

Attachment: Bug 1425088 - (Part 3) Put the dropdown frame's placeholder into the principal child list of the combobox frame when content-select is enabled.

Review commit: https://reviewboard.mozilla.org/r/210402/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210402/

Comment 6

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8940106 [details]
Bug 1425088 - (Part 2) Change the dropdown frame to a top-level absolute frame when content-select is enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210400/diff/1-2/

Comment 7

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8940107 [details]
Bug 1425088 - (Part 3) Put the dropdown frame's placeholder into the principal child list of the combobox frame when content-select is enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210402/diff/1-2/

Comment 8

[Mats Palmgren (inactive)]

Comment on attachment 8940105 [details]
Bug 1425088 - (Part 1) Add new pref to enable content-select.

https://reviewboard.mozilla.org/r/210398/#review216902

Comment 9

[Mats Palmgren (inactive)]

Comment on attachment 8940106 [details]
Bug 1425088 - (Part 2) Change the dropdown frame to a top-level absolute frame when content-select is enabled.

https://reviewboard.mozilla.org/r/210400/#review216904

::: commit-message-28f18:1
(Diff revision 2)
> +Bug 1425088 - (Part 2) Change the dropdown frame to a top-level absolute frame when content-select enabled.
> +

nit: s/enabled/is enabled/

::: layout/base/nsCSSFrameConstructor.cpp:138
(Diff revision 2)
>  using namespace mozilla;
>  using namespace mozilla::dom;
>  
>  // An alias for convenience.
>  static const nsIFrame::ChildListID kPrincipalList = nsIFrame::kPrincipalList;
> +static const char *kPrefSelectPopupInContent = "dom.select_popup_in_content.enabled";

nit: s/char */char* /

::: layout/style/res/forms.css:426
(Diff revision 2)
>    border: 1px outset black !important;
>    border-inline-start-width: 2px ! important;
>  }
>  
> +@supports -moz-bool-pref("dom.select_popup_in_content.enabled") {
> +  *|*::-moz-dropdown-list {

As far as I know, we only have -moz-dropdown-list pseudos for <select>, so this selector might be slightly faster:
select::-moz-dropdown-list

Comment 10

[Mats Palmgren (inactive)]

Comment on attachment 8940107 [details]
Bug 1425088 - (Part 3) Put the dropdown frame's placeholder into the principal child list of the combobox frame when content-select is enabled.

https://reviewboard.mozilla.org/r/210402/#review216910

::: commit-message-1cb89:1
(Diff revision 2)
> +Bug 1425088 - (Part 3) Put the dropdown frame into the principle child list of combobox frame when content-select is enabled.
> +

I think this message is a bit misleading since there are two frames involved - the placeholder and the absolute frame, and it's the placeholder that we put on the principal (not principle) child list, not the absolute frame (which I suspect most people would consider the "dropdown frame").
So I suggest:
Bug 1425088 - (Part 3) Put the dropdown frame's placeholder into the principal child list of the combobox frame when content-select is enabled.

Comment 11

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8940105 [details]
Bug 1425088 - (Part 1) Add new pref to enable content-select.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210398/diff/1-2/

Comment 12

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8940106 [details]
Bug 1425088 - (Part 2) Change the dropdown frame to a top-level absolute frame when content-select is enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210400/diff/2-3/

Comment 13

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8940107 [details]
Bug 1425088 - (Part 3) Put the dropdown frame's placeholder into the principal child list of the combobox frame when content-select is enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210402/diff/2-3/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0fc447f9723
(Part 1) Add new pref to enable content-select. r=mats
https://hg.mozilla.org/integration/autoland/rev/a395a065bdd3
(Part 2) Change the dropdown frame to a top-level absolute frame when content-select is enabled. r=mats
https://hg.mozilla.org/integration/autoland/rev/75edc4a8cbc3
(Part 3) Put the dropdown frame's placeholder into the principal child list of the combobox frame when content-select is enabled. r=mats

Comment 15

[Chih-Hsuan Kuo [:tommykuo]]

(In reply to Mats Palmgren (:mats) from comment #9)
> > +@supports -moz-bool-pref("dom.select_popup_in_content.enabled") {
> > +  *|*::-moz-dropdown-list {
> 
> As far as I know, we only have -moz-dropdown-list pseudos for <select>, so
> this selector might be slightly faster:
> select::-moz-dropdown-list

I found this will make Firefox crash when the pref is on, and the crash is happened here[1]. After I discussed with :heycam, he said that we only allow the anonymous box to use the universal selector. Because I already landed it, I'll change it back to the universal selector in Bug 1428960.

[1]: https://searchfox.org/mozilla-central/rev/cf149b7b63ff97023e28723167725e38cf5df757/servo/components/style/stylist.rs#2020

Comment 16

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/f0fc447f9723
https://hg.mozilla.org/mozilla-central/rev/a395a065bdd3
https://hg.mozilla.org/mozilla-central/rev/75edc4a8cbc3