Closed Bug 1425088 Opened 2 years ago Closed 2 years ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: kuoe0.tw, Assigned: kuoe0.tw)

References

Details

Attachments

(3 files)

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.
Assignee: nobody → kuoe0
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=
Flags: needinfo?(mats)
Priority: -- → P1
-moz-bool-pref works for me.
Flags: needinfo?(mats)
Attachment #8940105 - Flags: review?(mats)
Attachment #8940106 - Flags: review?(mats)
Attachment #8940107 - Flags: review?(mats)
Summary: [content-select] Change the dropdown frame of <select> to a top-level absolute frame → Change the dropdown frame of <select> to a top-level absolute frame
Component: Layout → Layout: Form Controls
Comment on attachment 8940105 [details]
Bug 1425088 - (Part 1) Add new pref to enable content-select.

https://reviewboard.mozilla.org/r/210398/#review216902
Attachment #8940105 - Flags: review?(mats) → review+
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
Attachment #8940106 - Flags: review?(mats) → review+
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.
Attachment #8940107 - Flags: review?(mats) → review+
Pushed by tokuo@mozilla.com:
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
(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
You need to log in before you can comment on or make changes to this bug.