Bug 1470880 - Simplify arrowpanel selectors in order to remove last usage of xbl resources. r=bgrins
☠☠ backed out by 54bfe0a47b67 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 22 Apr 2019 21:41:29 +0000
changeset 470414 9509ae2baf1cecd227c949c74bc4022b79e89459
parent 470413 8ff5048f2b90c941edb72bd671f3f728a4cdd15b
child 470415 73e8dcb8be078f4d66058019f70c2cd0e4526a6b
push id35905
push userdvarga@mozilla.com
push dateTue, 23 Apr 2019 09:53:27 +0000
treeherdermozilla-central@831918f009f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins
bugs1470880
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1470880 - Simplify arrowpanel selectors in order to remove last usage of xbl resources. r=bgrins The reason why this fixes it is a bit subtle, let me try to explain. XBL has this mechanism where all attributes in the binding `<content>` element get auto-propagated to the bound element (the `<panel>` in this case). This doesn't seem to be a very used feature looking at: https://searchfox.org/mozilla-central/search?q=%3Ccontent&case=false&regexp=false&path=xml The panel binding uses it to add the `side` attribute: https://searchfox.org/mozilla-central/rev/d80f0a570736dce76a2eb184fb65517462089e8a/toolkit/content/widgets/popup.xml#264 The key here is that this attribute addition is silent (`aNotify=false`): https://searchfox.org/mozilla-central/rev/d80f0a570736dce76a2eb184fb65517462089e8a/dom/xbl/nsXBLBinding.cpp#341 This means that the presence of this attribute is not supposed to change the rendering of the page. It'd also be unsafe to notify at the point at which we create XBL bindings. So the way this happens is: * We compute the initial style of the `<panel>` element (which doesn't have a `side` attribute, and thus doesn't match the rules, and has a computed opacity of 1). * The XBL service _silently_ sets the `side` attribute. This should cause a style change, but it doesn't since it's silent, so we remain with the opacity of 1. * We open the popup, and the XBL binding listens for the `popupshowing` event and adds the `animate` attribute. The style system notices, and eventually we compute the new style. Issue is, it has again an opacity of 1, so we don't fire the transition. Same with transform and such of course. So far so good, but then, why does it work if there's a `<resources>` element with an empty stylesheet? Fun that you ask! We explicitly re-resolve the style of the element if there are any stylesheets: https://searchfox.org/mozilla-central/rev/d80f0a570736dce76a2eb184fb65517462089e8a/dom/xbl/nsXBLService.cpp#551 And thus grab the correct initial opacity of zero, and trigger the transition. Given arrow panels always have a `side` attribute (and same for the bookmarks thing), making their style not depend on the silent attribute additions from `<content>` fixes the bug. We could fix the bug with an alternative patch (re-resolving style if there's a `<content>` element with attributes in the binding). This wouldn't be completely sound anyway in presence of combinators, and given that it'd remain being unsound anyway, we should probably just remove that feature. Also, the simplification of the stylesheets seems worth it anyway. I'll follow-up removing the `<resources>` implementation, and we should probably investigate removing the `<content>` attribute propagation, since it's the really unsound thing here. Differential Revision: https://phabricator.services.mozilla.com/D28310
browser/base/content/browser.css
toolkit/content/widgets/popup.xml
toolkit/content/xul.css
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -1101,17 +1101,17 @@ toolbarpaletteitem[place="palette"] > #d
   transition-timing-function:
     var(--animation-easing-function), ease-out;
 }
 
 #BMB_bookmarksPopup[side="bottom"]:not([animate="false"]) {
   -moz-window-transform: translateY(70px);
 }
 
-#BMB_bookmarksPopup[side][animate="open"] {
+#BMB_bookmarksPopup[animate="open"] {
   -moz-window-opacity: 1.0;
   transition-duration: 0.18s, 0.18s;
   -moz-window-transform: none;
   transition-timing-function:
     var(--animation-easing-function), ease-in-out;
 }
 
 #BMB_bookmarksPopup[animate="cancel"] {
@@ -1132,17 +1132,17 @@ toolbarpaletteitem[place="palette"] > #d
   transition-timing-function:
     var(--animation-easing-function), ease-out;
 }
 
 #BMB_bookmarksPopup[side="bottom"]:not([animate="false"]) {
   transform: translateY(70px);
 }
 
-#BMB_bookmarksPopup[side][animate="open"] {
+#BMB_bookmarksPopup[animate="open"] {
   opacity: 1.0;
   transition-duration: 0.18s, 0.18s;
   transform: none;
   transition-timing-function:
     var(--animation-easing-function), ease-in-out;
 }
 
 #BMB_bookmarksPopup[animate="cancel"] {
--- a/toolkit/content/widgets/popup.xml
+++ b/toolkit/content/widgets/popup.xml
@@ -251,21 +251,16 @@
             currentFocus = currentFocus.parentNode;
           }
         }
       ]]></handler>
     </handlers>
   </binding>
 
   <binding id="arrowpanel" extends="chrome://global/content/bindings/popup.xml#panel">
-    <resources>
-      <!-- Fixes an issue with the "test_arrowpanel.xul" animation on Mac, see bug 1470880. -->
-      <stylesheet src="data:text/css,"/>
-    </resources>
-
     <content flip="both" side="top" position="bottomcenter topleft" consumeoutsideclicks="false">
       <xul:vbox anonid="container" class="panel-arrowcontainer" flex="1"
                xbl:inherits="side,panelopen">
         <xul:box anonid="arrowbox" class="panel-arrowbox">
           <xul:image anonid="arrow" class="panel-arrow" xbl:inherits="side"/>
         </xul:box>
         <xul:box class="panel-arrowcontent" xbl:inherits="side,align,dir,orient,pack" flex="1">
           <children/>
--- a/toolkit/content/xul.css
+++ b/toolkit/content/xul.css
@@ -303,69 +303,69 @@ panel[type="arrow"] {
 /* On Mac, use the properties "-moz-window-transform" and "-moz-window-opacity"
    instead of "transform" and "opacity" for these animations.
    The -moz-window* properties apply to the whole window including the window's
    shadow, and they don't affect the window's "shape", so the system doesn't
    have to recompute the shadow shape during the animation. This makes them a
    lot faster. In fact, Gecko no longer triggers shadow shape recomputations
    for repaints.
    These properties are not implemented on other platforms. */
-panel[type="arrow"][side]:not([animate="false"]) {
+panel[type="arrow"]:not([animate="false"]) {
   -moz-window-opacity: 0;
   -moz-window-transform: translateY(-70px);
   transition-property: -moz-window-transform, -moz-window-opacity;
   transition-duration: 0.18s, 0.18s;
   transition-timing-function:
     var(--animation-easing-function), ease-out;
 }
 
 panel[type="arrow"][side="bottom"]:not([animate="false"]) {
   -moz-window-transform: translateY(70px);
 }
 
-panel[type="arrow"][side][animate="open"] {
+panel[type="arrow"][animate="open"] {
   -moz-window-opacity: 1.0;
   transition-duration: 0.18s, 0.18s;
   -moz-window-transform: none;
   transition-timing-function:
     var(--animation-easing-function), ease-in-out;
 }
 
-panel[type="arrow"][side][animate="cancel"] {
+panel[type="arrow"][animate="cancel"] {
   -moz-window-transform: none;
 }
 
 %elifndef MOZ_WIDGET_GTK
 
-panel[type="arrow"][side] {
+panel[type="arrow"] {
   will-change: transform, opacity; /* workaround for bug 1414033 */
 }
 
-panel[type="arrow"][side]:not([animate="false"]) {
+panel[type="arrow"]:not([animate="false"]) {
   opacity: 0;
   transform: translateY(-70px);
   transition-property: transform, opacity;
   transition-duration: 0.18s, 0.18s;
   transition-timing-function:
     var(--animation-easing-function), ease-out;
 }
 
 panel[type="arrow"][side="bottom"]:not([animate="false"]) {
   transform: translateY(70px);
 }
 
-panel[type="arrow"][side][animate="open"] {
+panel[type="arrow"][animate="open"] {
   opacity: 1.0;
   transition-duration: 0.18s, 0.18s;
   transform: none;
   transition-timing-function:
     var(--animation-easing-function), ease-in-out;
 }
 
-panel[type="arrow"][side][animate="cancel"] {
+panel[type="arrow"][animate="cancel"] {
   transform: none;
 }
 
 %endif
 panel[type="arrow"][animating] {
   pointer-events: none;
 }