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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-04-22 21:41:29 +00:00
Родитель 6b90ab2c92
Коммит e4f84a0b9d
3 изменённых файлов: 9 добавлений и 14 удалений

Просмотреть файл

@ -1106,7 +1106,7 @@ toolbarpaletteitem[place="palette"] > #downloads-button[indicator] > #downloads-
-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;
@ -1137,7 +1137,7 @@ toolbarpaletteitem[place="palette"] > #downloads-button[indicator] > #downloads-
transform: translateY(70px);
}
#BMB_bookmarksPopup[side][animate="open"] {
#BMB_bookmarksPopup[animate="open"] {
opacity: 1.0;
transition-duration: 0.18s, 0.18s;
transform: none;

Просмотреть файл

@ -256,11 +256,6 @@
</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">

Просмотреть файл

@ -308,7 +308,7 @@ panel[type="arrow"] {
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;
@ -321,7 +321,7 @@ 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;
@ -329,17 +329,17 @@ panel[type="arrow"][side][animate="open"] {
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;
@ -352,7 +352,7 @@ 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;
@ -360,7 +360,7 @@ panel[type="arrow"][side][animate="open"] {
var(--animation-easing-function), ease-in-out;
}
panel[type="arrow"][side][animate="cancel"] {
panel[type="arrow"][animate="cancel"] {
transform: none;
}