Closed Bug 1424095 Opened 7 years ago Closed 6 years ago

Tweak frontend CSS/JS to play nicely with modern-flexbox emulation for XUL

Categories

(Firefox :: General, enhancement, P3)

Product:
Firefox
Component:
General
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 60
Tracking Flags:
Tracking Status
firefox60 --- fixed

People

(Reporter: dholbert, Assigned: bgrins)

References

Details

Attachments

(3 files, 5 obsolete files)

Bug 1424095 - Tweak frontend CSS to play nicely in XUL flexbox emulation mode;
59 bytes, text/x-review-board-request
Gijs
: review+
Details
Bug 1424095 - Use display: block on iframes in XUL flexbox emulation mode;
59 bytes, text/x-review-board-request
Gijs
: review+
Details
Bug 1424095 - Force off flexbox emulation in the Browser Toolbox;
59 bytes, text/x-review-board-request
jryans
: review+
Details 
I'm filing this bug to cover the frontend JS/CSS changes that are necessary to make our modern-flexbox-based emulation of XUL work nicely.

Until now, we've been lumping these changes into the patch stack on bug 1398963; but I'd like to split them out away from that bug, becausse I intend to use that bug to land the (preffed-off) C++ changes associated with XUL emulation.  After that lands, we can sort out the CSS/JS changes here (with those maybe also being landable behind a pref, pending bug 1267890).
Attached patch initial WIP patch from other bug (obsolete) — — Splinter Review 
To kick things off, here's a patch with the (unbitrotted) frontend changes from the latest patch over on bug 1398963 (which I'll be stomping on over on that bug shortly).

In other words, this is https://reviewboard-hg.mozilla.org/gecko/rev/87f3bbd6ee0ac1aa6505f9cbbb2c252a25e01c0f , unbitrotted, minus the C++ changes.
Attachment #8935574 - Attachment is obsolete: true
I reposted an unbitrotted patch here, now using MozReview.  I also just posted a logically-split-up patch queue over on bug 1398963.

To test this emulation feature, you need to apply both sets of patches[1], AND you need to toggle the pref layout.css.emulate-moz-box-with-flex to true.

(The current patches on bug 1398963 will make this pref show up in about:config, in prerelease builds, but it defaults to false.)


[1] (RE applying both sets of patches -- you can also just use the "hg pull" command on this bug's MozReview page, because this bug's patch includes the other bug's patches in its ancestor-chain.)
(To be clear -- right now the pref only turns the C++ emulation on/off, but ultimately we'll want to make some or all of the frontend-CSS-changes depend on that pref too.)
Blocks: 1425330
Rebased and added the @supports rule for the ones that obviously need it. Ideally we can land some of the changes unconditionally, but we'll have to review them to make sure it doesn't change the default layout.
Attachment #8936865 - Attachment is obsolete: true
Did a bit of cleanup and added a few comments.  Now doing a mozscreenshots run to confirm we are about where we left off in the previous bug.
bgrins and I identified a few remaining issues with getting the UI to parity with this pref enabled.

Higher priority:
================
 (1) Tabs with large titles don't shrink (down to 75px) like they do in unmodified Nightly.

 (2) Pinned tabs are too tall (visually, you can see this because the stripe to the right of the pinned tab protrudes downwards into the toolbar below it -- but really the whole <tab> is a few px too tall). This has something to do with the tab-loading-burst element, which seems to make its container a few pixels taller. Might be a weird <stack> interaction.

Lower priority:
===============
 (3) Splitters aren't draggable -- e.g. the dividing line between the page area & the bookmarks sidebar (Ctrl+B) or devtools (F12)

 (4) The browser console isn't scrollable (likely needs min-height:0 somewhere -- though I think bgrins said that's being replaced/reimplemented soon, so this isn't a priority)


I'm investigating (1) right now -- current status & thoughts:
Testcase: data:text/html,<title>aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
Fixes needed: at a minimum, we need to add min-width:0 to .tab-stack here:
 https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/tabs.inc.css#45
We also need to give the <tab> parent (a xul:box) the style "max-width:100%", I think, to prevent it from gracefully shrinkwrapping all of the tabs' min/max clamped preferred sizes.  Otherwise, we end up with wide tab-titles contributing their max-width (225px) to the scrollbox-innerbox's auto-size, and then unsurprisingly the scrollbox-innerbox ends up giving them that size back when it does layout, even if we've got zillions of tabs.

With that, the tabstrip mostly sizes the same as it does in Nightly, though there's still one difference -- if you have a mix of wide-title and skinny-title tabs and there's enough room for them all to fit and grow a little, then a Flex Emulation build will give the extra space *preferentially* to the tabs with wider tab-titles. Whereas in Nightly, the extra space is distributed evenly to all tabs.  I think that's due to a difference in "flex-basis" behavior between XUL and flexbox -- that doesn't really have a direct representation in XUL.  The simplest solution here might just be to go *fully* (not emulated) display:flex for this container, and give the tabs flex:1 (i.e. flex:1 1 0px) so that they share free space 100% evenly with flex-basis being explicitly zero... Though that switchover will require further tweaks, it seems.
(Here's a patch with the first two suggested tweaks from the end of comment 10 - this lets wide-title tabs shrink down to the correct minimum width, though space still isn't always distributed equally between wide-title & skinny-title tabs in a-little-extra-space scenarios as noted later on in comment 10.)
Comment on attachment 8937830 [details]
Bug 1424095 - Tweak frontend CSS to play nicely in XUL flexbox emulation mode;

https://reviewboard.mozilla.org/r/208530/#review216974

::: browser/base/content/browser.css:133
(Diff revision 3)
> +@supports -moz-bool-pref("layout.css.emulate-moz-box-with-flex") {
> +  .tabbrowser-tab:not([pinned]) {
> +    /* Without this, tabs appear misaligned when overflowing */
> +    width: auto;
> +  }
> +}

I think this chunk needs to be removed -- it's causing the wide-tabs-not-shrinking-fairly issue at the very end of comment 10.  If I remove this and apply attachment 8940860 [details] [diff] [review] (my "extra patch"), then tabs seem to get the correct (and fairly-distributed) widths, AFAICT.  Hooray!

(I'm guessing the "tabs appear misaligned" problem hinted at in the code-comment here is separately-addressed by my "extra patch", or maybe it's something that's gone away for other reasons. Or maybe I'm just not noticing the misalignment.)
(In reply to Daniel Holbert [:dholbert] from comment #12)
> Comment on attachment 8937830 [details]
> Bug 1424095: WIP patch to tweak frontend CSS to play nicely with XUL being
> emulated via modern flexbox.
> 
> https://reviewboard.mozilla.org/r/208530/#review216974
> 
> ::: browser/base/content/browser.css:133
> (Diff revision 3)
> > +@supports -moz-bool-pref("layout.css.emulate-moz-box-with-flex") {
> > +  .tabbrowser-tab:not([pinned]) {
> > +    /* Without this, tabs appear misaligned when overflowing */
> > +    width: auto;
> > +  }
> > +}
> 
> I think this chunk needs to be removed -- it's causing the
> wide-tabs-not-shrinking-fairly issue at the very end of comment 10.  If I
> remove this and apply attachment 8940860 [details] [diff] [review] (my
> "extra patch"), then tabs seem to get the correct (and fairly-distributed)
> widths, AFAICT.  Hooray!
> 
> (I'm guessing the "tabs appear misaligned" problem hinted at in the
> code-comment here is separately-addressed by my "extra patch", or maybe it's
> something that's gone away for other reasons.

That sounds plausible - feel free to remove this and then fold your fixes into part 1 and obsolete my patch.
bgrins, could you trigger a screenshots-comparison run for recent trunk with the latest attachment that I just pushed to MozReview? (or show me how to do so myself sometime tomorrow maybe?)  I'd like to compare builds with/without the pref set. (Or ~equivalently, compare unmodified m-c to m-c with the patch *and* the pref set.)

I think this latest attachment addresses both (1) and (2) from comment 10, so I'm curious to see if screenshots can confirm that & if anything else stands out.
Attachment #8940860 - Attachment is obsolete: true
Attachment #8937830 - Attachment is obsolete: true
Flags: needinfo?(bgrinstead)
Mozscreenshots with patch but no pref flip (no relevant changes, as expected): https://screenshots.mattn.ca/compare/?oldProject=try&oldRev=04d90f7b9d7a3b59815ddf1684090dcfa04ed190&newProject=try&newRev=5bae1afdaabfec18ec83ada4aa46de5ff67d0dfe

Mozscreenshots with patch + pref flip does show changes, but luckily the pinned tabs issue seems fixed: https://screenshots.mattn.ca/compare/?oldProject=try&oldRev=04d90f7b9d7a3b59815ddf1684090dcfa04ed190&newProject=try&newRev=5bae1afdaabfec18ec83ada4aa46de5ff67d0dfe.

A couple of things I see (note that not all of the 'compare' links are currently working on that page so I've had to manually click before/after):

1) Control center panel is a bit shorter.  I think we knew about this and were OK with it (as the CSS flex is actually fixing a bug that we are working around for XUL flex).
2) On Windows: it appears the content is shifted up with the patch applied:
** Base: https://public-artifacts.taskcluster.net/LyCt2rvbTpu0wYQOJaXwfg/0/public/test_info/20180123100248-primaryUI_091_tabsInTitlebar_fourPinned_maximized_onlyNavBar_noLWT_compactDensity.png
** New: https://public-artifacts.taskcluster.net/DLhRMX_wSfqtq_Vzlgu1sA/0/public/test_info/20180123100248-primaryUI_091_tabsInTitlebar_fourPinned_maximized_onlyNavBar_noLWT_compactDensity.png
** Compare: https://screenshots.mattn.ca/comparisons/try/04d90f7b9d7a3b59815ddf1684090dcfa04ed190/try/5bae1afdaabfec18ec83ada4aa46de5ff67d0dfe/windows10-64/primaryUI_091_tabsInTitlebar_fourPinned_maximized_onlyNavBar_noLWT_compactDensity.png
Flags: needinfo?(bgrinstead)
(In reply to Brian Grinstead [:bgrins] from comment #16)
> Mozscreenshots with patch but no pref flip (no relevant changes, as
> expected):
> https://screenshots.mattn.ca/compare/
> ?oldProject=try&oldRev=04d90f7b9d7a3b59815ddf1684090dcfa04ed190&newProject=tr
> y&newRev=5bae1afdaabfec18ec83ada4aa46de5ff67d0dfe

Copy-pasted the same link twice. Here's the right link for "with patch but no pref flip": https://screenshots.mattn.ca/compare/?oldProject=try&oldRev=04d90f7b9d7a3b59815ddf1684090dcfa04ed190&newProject=try&newRev=1e8701b33c85479801b88645f99fe28ca6bf3fe5
(In reply to Brian Grinstead [:bgrins] from comment #16)
> 1) Control center panel is a bit shorter.  I think we knew about this and
> were OK with it (as the CSS flex is actually fixing a bug that we are
> working around for XUL flex).

That matches my recollection, yeah.

> 2) On Windows: it appears the content is shifted up with the patch applied:

Specifically when fullscreen, I think.  We saw this earlier in bug 1398963 comment 39, and it was due to some weirdness with a windows-specific pseudoclass that applies when you maximize (or fullscreen?) the window. Specifically, I think we have some frontend CSS that gives a negative "margin-top" to the tabstrip, and that's intended to counteract some piece of system-imposed window-decoration that otherwise shows up there.  But we don't need that negative margin-top (or it's too negative) when flex emulation is applied, for some reason. I can take another look at this, though I'm not sure it needs to block this (preffed-off) landing.
Attachment #8944652 - Attachment is obsolete: true
Blocks: 1432648
Just for fun, here's a mochitest try push with the patch and the pref flipped: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a4b04c34aa7586aeef40e6de1b60b9354d607af&group_state=expanded.

There's a couple of green bc chunks in linux debug which is promising. I wonder how much green we'd have with iframe sizing and splitters fixed (I expect most of the dt failures are due to the toolbox being tiny and content not being visible).
Comment on attachment 8945221 [details]
Bug 1424095 - Force off flexbox emulation in the Browser Toolbox;

https://reviewboard.mozilla.org/r/215450/#review221060

Seems logical! :)
Attachment #8945221 - Flags: review?(jryans) → review+
Comment on attachment 8945219 [details]
Bug 1424095 - Make devtools toolbox sizing use CSS properties for width/height instead of using XUL attributes;

https://reviewboard.mozilla.org/r/215446/#review221072


Static analysis found 5 defects in this patch.
 - 5 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint check path/to/file` (Python/Javascript/wpt)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/client/framework/toolbox-hosts.js:174
(Diff revision 1)
>     */
>    destroy: function () {
>      if (!this._destroyed) {
>        this._destroyed = true;
>  
> -      Services.prefs.setIntPref(this.heightPref, this.frame.height);
> +      Services.prefs.setIntPref(this.heightPref, parseInt(this.frame.style.height));

Error: Missing radix parameter. [eslint: radix]

::: devtools/client/framework/toolbox-hosts.js:260
(Diff revision 1)
>     */
>    destroy: function () {
>      if (!this._destroyed) {
>        this._destroyed = true;
>  
> -      Services.prefs.setIntPref(this.widthPref, this.frame.width);
> +      Services.prefs.setIntPref(this.widthPref, parseInt(this.frame.style.width));

Error: Missing radix parameter. [eslint: radix]

::: devtools/client/framework/toolbox.js:504
(Diff revision 1)
>        }
>  
>        this._componentMount.addEventListener("keypress", this._onToolbarArrowKeypress);
>  
>        this.webconsolePanel = this.doc.querySelector("#toolbox-panel-webconsole");
> -      this.webconsolePanel.height = Services.prefs.getIntPref(SPLITCONSOLE_HEIGHT_PREF);
> +      this.webconsolePanel.style.height = Services.prefs.getIntPref(SPLITCONSOLE_HEIGHT_PREF) + "px";

Error: Line 504 exceeds the maximum line length of 90. [eslint: max-len]

::: devtools/client/framework/toolbox.js:937
(Diff revision 1)
>    _registerOverlays: function () {
>      registerHarOverlay(this);
>    },
>  
>    _saveSplitConsoleHeight: function () {
> -    Services.prefs.setIntPref(SPLITCONSOLE_HEIGHT_PREF,
> +    Services.prefs.setIntPref(SPLITCONSOLE_HEIGHT_PREF, parseInt(this.webconsolePanel.style.height));

Error: Line 937 exceeds the maximum line length of 90. [eslint: max-len]

::: devtools/client/framework/toolbox.js:937
(Diff revision 1)
>    _registerOverlays: function () {
>      registerHarOverlay(this);
>    },
>  
>    _saveSplitConsoleHeight: function () {
> -    Services.prefs.setIntPref(SPLITCONSOLE_HEIGHT_PREF,
> +    Services.prefs.setIntPref(SPLITCONSOLE_HEIGHT_PREF, parseInt(this.webconsolePanel.style.height));

Error: Missing radix parameter. [eslint: radix]
Just a note about the iframe sizing in global.css: the `width: 100px; height: 100px;` was added in 2004 and has remained unchanged since then (bug 247631 /  https://github.com/mozilla/gecko-dev/commit/e5632337c122eac31fbc85d2a5cba910448733fe). Instead of resetting width and height when emulating we could check to see if the hardcoded with/height is still necessary and remove it if not.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Comment on attachment 8945219 [details]
Bug 1424095 - Make devtools toolbox sizing use CSS properties for width/height instead of using XUL attributes;

https://reviewboard.mozilla.org/r/215446/#review221084

The patch is not completely working for me.
The toolbox height is not saved after closing and reopening the toolbox. STRs:
- open toolbox docked in bottom
- resize the browser/devtools splitter to make the toolbox bigger
- close/open toolbox
=> size was not saved 

The issue is that the splitter between devtools and the browser is still setting the XUL height attribute on the iframe.
The iframe style.height still has the initial value when we close the toolbox.

::: devtools/client/framework/toolbox-hosts.js:71
(Diff revision 2)
>      // Avoid resizing notification containers
>      this._splitter.setAttribute("resizebefore", "flex");
>  
>      this.frame = ownerDocument.createElement("iframe");
>      this.frame.className = "devtools-toolbox-bottom-iframe";
> -    this.frame.height = Math.min(
> +    this.frame.style.height = Math.min(

this.frame.height is still used in minimize(), should use parseInt(this.frame.style.height, 10) instead ? 

(sidenote: minimize() is not really used in practice, I should open a RFC to remove the related code)
Attachment #8945219 - Flags: review?(jdescottes)
Comment on attachment 8937830 [details]
Bug 1424095 - Tweak frontend CSS to play nicely in XUL flexbox emulation mode;

https://reviewboard.mozilla.org/r/208530/#review221236

This looks OK to me from a code perspective; I haven't actually run this... I'm basically r+'ing on the basis that we're going to be iterating on this, and this certainly doesn't look objectionable as-is, though I expect there'll be more tweaks to come. Do we think we can complete this transition for 60?

::: browser/themes/shared/controlcenter/panel.inc.css:316
(Diff revision 6)
>  
> +@supports -moz-bool-pref("layout.css.emulate-moz-box-with-flex") {
> +  /* The extra padding-bottom is there to work around XUL flex (Bug 1368281).
> +     This rule and the 1.5em above can both be removed once we are only using CSS flex. */
> +  #identity-popup-permissions-content {
> +    padding-bottom: 1em;

Could we use `padding-bottom: unset` here?

::: toolkit/content/xul.css:1085
(Diff revision 6)
> +  [pack="end"] { -moz-box-pack: end; }
> +
> +  /* This isn't a real solution for [flex] and [ordinal], but it covers enough
> +     cases to render the browser chrome. If we get attr() in Bug 435426 this could
> +     work for all cases. */
> +  [flex="1"] { -moz-box-flex: 1; }

So, I'm curious, I thought this bug was switching us over to standards flex. Why isn't this using flex-grow/shrink/basis, then, and still using box-flex ?

::: toolkit/content/xul.css:1094
(Diff revision 6)
> +  [flex="400"] { -moz-box-flex: 400; }
> +  [flex="1000"] { -moz-box-flex: 1000; }

This is amazing. Also, I think we use flex="100" in some places, notably the search bar, so we should probably include that...
Attachment #8937830 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8945220 [details]
Bug 1424095 - Use display: block on iframes in XUL flexbox emulation mode;

https://reviewboard.mozilla.org/r/215448/#review221240

Happy with either this or removal. Removal might require updating some automated (ref?)tests that are too lazy to set default sizes.
Attachment #8945220 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs (lower availability until Jan 27) from comment #34)
> Comment on attachment 8945220 [details]
> Bug 1424095 - Use display: block on iframes in XUL flexbox emulation mode;
> 
> https://reviewboard.mozilla.org/r/215448/#review221240
> 
> Happy with either this or removal. Removal might require updating some
> automated (ref?)tests that are too lazy to set default sizes.

Thanks, I'll file a followup bug to look into removing the default sizes
(In reply to :Gijs (lower availability until Jan 27) from comment #33)
> Do we think we can complete this transition for 60?

I would not make any promises about that. :)  Right now this is for prototyping/experimentation.

> > +  [flex="1"] { -moz-box-flex: 1; }
> 
> So, I'm curious, I thought this bug was switching us over to standards flex.
> Why isn't this using flex-grow/shrink/basis, then, and still using box-flex ?

This is still using box-flex because the elements in quesiton (e.g. <hbox flex="1"> etc) are still display:-moz-box (which is applied in xul.css or somesuch), which means they're still sized using -moz-box-flex.  The "flex" properties intentionally have no effect on these elements, regardless of emulation.

This bug and its associated bugs are part of experimenting with making XUL <box> & its associated properties be *implemented under the hood* using the same C++ code as our modern flexbox implementation (nsFlexContainerFrame.cpp), without need for (much) manual CSS rewriting.  This is a stepping stone towards maybe eventually styling these elements with "display:flex".  But for now, they are still "display:-moz-box" and hence are sized/positioned/etc. with the old-style properties (and the cool thing is that with the pref flipped, we'll be using nsFlexContainerFrame.cpp *to implement* the old properties under the hood, rather than using /layout/xul/ C++ code).  See for example https://dxr.mozilla.org/mozilla-central/rev/e22e2c9eb81686e958a9448ea3d1e8cd766743e2/layout/generic/nsFlexContainerFrame.cpp#1219-1229
(In reply to Daniel Holbert [:dholbert] from comment #36)
> (In reply to :Gijs (lower availability until Jan 27) from comment #33)
> > Do we think we can complete this transition for 60?
> 
> I would not make any promises about that. :)  Right now this is for
> prototyping/experimentation.

Yeah - based on initial talos pushes there's going to be quite a bit of perf work needed before we can think about switching over to use CSS flex in the frontend. One bonus of such perf work though is that it also helps out web content (and features in the chrome that use CSS flex like the webconsole). Once this bug is done we can start tracking that in Bug 1425330.
See Also: → 1433295
(In reply to Julian Descottes [:jdescottes][:julian] from comment #32)
> Comment on attachment 8945219 [details]
> Bug 1424095 - Make devtools toolbox sizing use CSS properties for
> width/height instead of using XUL attributes;
> 
> https://reviewboard.mozilla.org/r/215446/#review221084
> 
> The patch is not completely working for me.
> The toolbox height is not saved after closing and reopening the toolbox.
> STRs:
> - open toolbox docked in bottom
> - resize the browser/devtools splitter to make the toolbox bigger
> - close/open toolbox
> => size was not saved 
> 
> The issue is that the splitter between devtools and the browser is still
> setting the XUL height attribute on the iframe.
> The iframe style.height still has the initial value when we close the
> toolbox.

Ah thanks, I missed that when testing locally with the pref flipped because <splitter> doesn't work in emulation mode. And the mochitest missed it because it doesn't actually click and drag the splitter but rather directly sets the height. Hm... OK, I might spin this off into another bug so we can more thoroughly investigate any differences between height and style.height wrt splitter.

> ::: devtools/client/framework/toolbox-hosts.js:71
> (Diff revision 2)
> >      // Avoid resizing notification containers
> >      this._splitter.setAttribute("resizebefore", "flex");
> >  
> >      this.frame = ownerDocument.createElement("iframe");
> >      this.frame.className = "devtools-toolbox-bottom-iframe";
> > -    this.frame.height = Math.min(
> > +    this.frame.style.height = Math.min(
> 
> this.frame.height is still used in minimize(), should use
> parseInt(this.frame.style.height, 10) instead ? 
> 
> (sidenote: minimize() is not really used in practice, I should open a RFC to
> remove the related code)

Yeah, I'll update that code in place and push it back up, at least for future reference.
Attachment #8945219 - Attachment is obsolete: true
Attachment #8945219 - Flags: review?(jdescottes)
Comment on attachment 8945219 [details]
Bug 1424095 - Make devtools toolbox sizing use CSS properties for width/height instead of using XUL attributes;

https://reviewboard.mozilla.org/r/215446/#review221486


Static analysis found 1 defect in this patch.
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint check path/to/file` (Python/Javascript/wpt)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: devtools/client/framework/toolbox-hosts.js:125
(Diff revision 3)
>  
>        this.frame.removeEventListener("transitionend", onTransitionEnd);
>        this.emit("minimized");
>      };
>      this.frame.addEventListener("transitionend", onTransitionEnd);
> -    this.frame.style.marginBottom = -this.frame.height + height + "px";
> +    this.frame.style.marginBottom = (height - parseInt(this.frame.style.height, 10)) + "px";

Error: Line 125 exceeds the maximum line length of 90. [eslint: max-len]
(In reply to :Gijs (lower availability until Jan 27) from comment #33)
> ::: browser/themes/shared/controlcenter/panel.inc.css:316
> (Diff revision 6)
> >  
> > +@supports -moz-bool-pref("layout.css.emulate-moz-box-with-flex") {
> > +  /* The extra padding-bottom is there to work around XUL flex (Bug 1368281).
> > +     This rule and the 1.5em above can both be removed once we are only using CSS flex. */
> > +  #identity-popup-permissions-content {
> > +    padding-bottom: 1em;
> 
> Could we use `padding-bottom: unset` here?

I just checked, and I don't think this'll work for padding. We are basically in this situation right now with the current patch and emulation on:

data:text/html,<style>div { background: red; padding-bottom: 1em;} div { padding-bottom: 1.5em; } div { padding-bottom: 1em; }</style><div></div>

If we try to use unset, it just resets padding-bottom to 0 instead of 1em:

data:text/html,<style>div { background: red; padding-bottom: 1em;} div { padding-bottom: 1.5em; } div { padding-bottom: unset; }</style><div></div>

Let me know if there's something else to do to make this work. Ideally when we ship with CSS flex we can remove both the offending rule at (which is in place to work around a XUL flex bug) at https://searchfox.org/mozilla-central/rev/062e1cf551f5bf3f0af33671b818f75a55ac497b/browser/themes/shared/controlcenter/panel.inc.css#309, and this one as well.

> ::: toolkit/content/xul.css:1094
> (Diff revision 6)
> > +  [flex="400"] { -moz-box-flex: 400; }
> > +  [flex="1000"] { -moz-box-flex: 1000; }
> 
> This is amazing. Also, I think we use flex="100" in some places, notably the
> search bar, so we should probably include that...

Thanks! Missed that one in my search (there were many more results for "flex=" than "ordinal=").
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/738845c4c146
Tweak frontend CSS to play nicely in XUL flexbox emulation mode;r=Gijs
https://hg.mozilla.org/integration/autoland/rev/6c8c4e5b32c4
Use display: block on iframes in XUL flexbox emulation mode;r=Gijs
https://hg.mozilla.org/integration/autoland/rev/c20dd87101e6
Force off flexbox emulation in the Browser Toolbox;r=jryans
Depends on: 1433420
Blocks: 1434080
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: