Bug 306344 - Use transform animations to implement marquee. r=smaug,f…

…irefox-animation-reviewers,boris This matches what chromium does, and is both simpler and prettier. The overflow: hidden enforcement also matches chromium / WebKit via: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/resolver/style_adjuster.cc;l=482;drc=88c8510c16d44e0dc8c07426db31aa5bb3c90a2b https://searchfox.org/wubkat/rev/473ca5f8512b88edd7e82c8783e7e09158f17ba1/Source/WebCore/style/StyleAdjuster.cpp#581-596 See also whatwg/html#10243 and the resets.html change. Adding white-space: nowrap isn't strictly necessary, but also matches other implementations: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/html_marquee_element.cc;l=66;drc=4aba604f0aae6fb93eab830b68765604e9a2cca0 (and same link as above on WebKit) Differential Revision: https://phabricator.services.mozilla.com/D206357
erichdongubler-mozilla · Apr 9, 2024 · 689cc63 · 689cc63
1 parent 232a664
commit 689cc63
Show file tree

Hide file tree

Showing 17 changed files with 291 additions and 359 deletions.
diff --git a/dom/html/HTMLMarqueeElement.cpp b/dom/html/HTMLMarqueeElement.cpp
@@ -107,19 +107,6 @@ bool HTMLMarqueeElement::ParseAttribute(int32_t aNamespaceID,
                                               aMaybeScriptedPrincipal, aResult);
 }
 
-void HTMLMarqueeElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
-                                      const nsAttrValue* aValue,
-                                      const nsAttrValue* aOldValue,
-                                      nsIPrincipal* aMaybeScriptedPrincipal,
-                                      bool aNotify) {
-  if (IsInComposedDoc() && aNameSpaceID == kNameSpaceID_None &&
-      aName == nsGkAtoms::direction) {
-    NotifyUAWidgetSetupOrChange();
-  }
-  return nsGenericHTMLElement::AfterSetAttr(
-      aNameSpaceID, aName, aValue, aOldValue, aMaybeScriptedPrincipal, aNotify);
-}
-
 void HTMLMarqueeElement::MapAttributesIntoRule(
     MappedDeclarationsBuilder& aBuilder) {
   nsGenericHTMLElement::MapImageMarginAttributeInto(aBuilder);

diff --git a/dom/html/HTMLMarqueeElement.h b/dom/html/HTMLMarqueeElement.h
@@ -102,10 +102,6 @@ class HTMLMarqueeElement final : public nsGenericHTMLElement {
                       const nsAString& aValue,
                       nsIPrincipal* aMaybeScriptedPrincipal,
                       nsAttrValue& aResult) override;
-  void AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName,
-                    const nsAttrValue* aValue, const nsAttrValue* aOldValue,
-                    nsIPrincipal* aMaybeScriptedPrincipal,
-                    bool aNotify) override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
   nsMapRuleToAttributesFunc GetAttributeMappingFunction() const override;
 

diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp
@@ -1265,17 +1265,7 @@ nsMargin nsHTMLScrollFrame::ComputeStableScrollbarGutter(
 
 // Legacy, this sucks!
 static bool IsMarqueeScrollbox(const nsIFrame& aScrollFrame) {
-  if (!aScrollFrame.GetContent()) {
-    return false;
-  }
-  if (MOZ_LIKELY(!aScrollFrame.GetContent()->HasBeenInUAWidget())) {
-    return false;
-  }
-  MOZ_ASSERT(aScrollFrame.GetParent() &&
-             aScrollFrame.GetParent()->GetContent());
-  return aScrollFrame.GetParent() &&
-         HTMLMarqueeElement::FromNodeOrNull(
-             aScrollFrame.GetParent()->GetContent());
+  return HTMLMarqueeElement::FromNodeOrNull(aScrollFrame.GetContent());
 }
 
 /* virtual */

diff --git a/layout/painting/crashtests/crashtests.list b/layout/painting/crashtests/crashtests.list
@@ -18,7 +18,7 @@ load 1469472.html
 load 1477831-1.html
 load 1504033.html
 load 1514544-1.html
-asserts(0-1) asserts-if(Android,0-2) load 1547420-1.html
+asserts(0-5) load 1547420-1.html
 load 1549909.html
 load 1551389-1.html
 asserts(0-2) load 1555819-1.html

diff --git a/layout/reftests/bugs/404553-1-ref.html b/layout/reftests/bugs/404553-1-ref.html
@@ -1 +1 @@
-<div style="background-color: lime; height: 50px;"><div style="background: green; width: 50px; margin-left: auto; margin-right: 0;">&nbsp;</div></div>
+<div style="background-color: lime; height: 50px;"><div style="background: green; width: 50px;">&nbsp;</div></div>
diff --git a/layout/reftests/bugs/404553-1.html b/layout/reftests/bugs/404553-1.html
@@ -1 +1 @@
-<table><marquee behavior="alternate" scrollamount="0" style="background-color: lime; height: 50px;"><div style="background: green; width: 50px">&nbsp;</div></marquee><span><title>
+<table><marquee behavior="alternate" scrollamount="0" style="background-color: lime; height: 50px;"><div style="background: green; width: 50px">&nbsp;</div></marquee><span><title>
diff --git a/layout/reftests/marquee/336736-1a-ref.html b/layout/reftests/marquee/336736-1a-ref.html
@@ -0,0 +1,5 @@
+<html>
+<body dir="rtl">
+<div style="background: green; width: 50px">&nbsp;</div>
+</body>
+</html>
diff --git a/layout/reftests/marquee/336736-1-ref.html → layout/reftests/marquee/336736-1b-ref.html b/layout/reftests/marquee/336736-1-ref.html → layout/reftests/marquee/336736-1b-ref.html
diff --git a/layout/reftests/marquee/reftest.list b/layout/reftests/marquee/reftest.list
@@ -1,6 +1,6 @@
 == 166591-dynamic-1.html 166591-dynamic-1-ref.html
-fuzzy-if(Android,0-8,0-50) == 336736-1a.html 336736-1-ref.html
-fuzzy-if(Android,0-8,0-50) == 336736-1b.html 336736-1-ref.html
+fuzzy-if(Android,0-8,0-50) == 336736-1a.html 336736-1a-ref.html
+fuzzy-if(Android,0-8,0-50) == 336736-1b.html 336736-1b-ref.html
 == 406073-1.html 406073-1-ref.html
 == 407016-2.html 407016-2-ref.html
 fuzzy-if(Android,0-8,0-220) == 413027-4.html 413027-4-ref.html

diff --git a/layout/style/res/html.css b/layout/style/res/html.css
@@ -816,15 +816,21 @@ dialog::backdrop {
   background: rgba(0, 0, 0, 0.1);
 }
 
+/* https://html.spec.whatwg.org/#the-marquee-element-2 */
 marquee {
-  inline-size: -moz-available;
   display: inline-block;
+  text-align: initial;
+  overflow: hidden !important;
+
+  /* See https://github.com/whatwg/html/issues/10249 */
+  inline-size: -moz-available;
   vertical-align: text-bottom;
-  text-align: start;
+  white-space: nowrap;
 }
 
 marquee:is([direction="up"], [direction="down"]) {
   block-size: 200px;
+  white-space: unset;
 }
 
 /* Ruby */

diff --git a/...tml/obsolete/requirements-for-implementations/the-marquee-element-0/marquee-loop.html.ini b/...tml/obsolete/requirements-for-implementations/the-marquee-element-0/marquee-loop.html.ini
diff --git a/...lete/requirements-for-implementations/the-marquee-element-0/marquee-scrollamount.html.ini b/...lete/requirements-for-implementations/the-marquee-element-0/marquee-scrollamount.html.ini
diff --git a/...olete/requirements-for-implementations/the-marquee-element-0/marquee-scrolldelay.html.ini b/...olete/requirements-for-implementations/the-marquee-element-0/marquee-scrolldelay.html.ini
diff --git a/...tml/obsolete/requirements-for-implementations/the-marquee-element-0/marquee-overflow.html b/...tml/obsolete/requirements-for-implementations/the-marquee-element-0/marquee-overflow.html
@@ -0,0 +1,18 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Marquee forces overflow: hidden</title>
+<link rel="help" href="https://html.spec.whatwg.org/#the-marquee-element">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<marquee style="overflow: visible">&nbsp;</marquee>
+<marquee style="overflow: scroll">&nbsp;</marquee>
+<marquee style="overflow: clip">&nbsp;</marquee>
+<marquee style="overflow: auto">&nbsp;</marquee>
+
+<script>
+test(() => {
+  for (let m of document.querySelectorAll("marquee")) {
+    assert_equals(getComputedStyle(m).overflow, "hidden", m.style);
+  }
+}, "Marquee should have overflow: hidden !important in the UA stylesheet");
+</script>
diff --git a/testing/web-platform/tests/html/rendering/non-replaced-elements/form-controls/resets.html b/testing/web-platform/tests/html/rendering/non-replaced-elements/form-controls/resets.html
@@ -50,6 +50,7 @@
 }
 input[type=hidden i] { display: none !important; }
 marquee {
+  overflow: hidden;
   text-align: initial;
 }
 table { display: table; box-sizing: border-box; }

diff --git a/toolkit/content/widgets/marquee.css b/toolkit/content/widgets/marquee.css
@@ -2,25 +2,19 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-.outerDiv {
-  overflow: hidden;
-  width: -moz-available;
+slot {
+  display: block;
+  will-change: translate;
 }
 
-.horizontal > .innerDiv {
-  width: max-content;
-  /* We want to create overflow of twice our available space. */
-  padding: 0 100%;
-}
-
-/* disable scrolling in contenteditable */
-:host(:read-write) .innerDiv {
-  padding: 0 !important;
+/* Disable the animation on contenteditable */
+:host(:read-write) > slot {
+  translate: none !important;
 }
 
 /* When printing or when the user doesn't want movement, we disable scrolling */
 @media print, (prefers-reduced-motion) {
-  .innerDiv {
-    padding: 0 !important;
+  slot {
+    translate: none !important;
   }
 }