Bug 1805105 - Invalidate paint for <input type=range> when its @list changes r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D164424

layout/forms/ListMutationObserver.cpp

@@ -0,0 +1,92 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+#include "ListMutationObserver.h"
+
+#include "mozilla/dom/HTMLInputElement.h"
+#include "nsIFrame.h"
+
+namespace mozilla {
+NS_IMPL_ISUPPORTS(ListMutationObserver, nsIMutationObserver)
+
+ListMutationObserver::~ListMutationObserver() = default;
+
+void ListMutationObserver::Attach(bool aRepaint) {
+  nsAutoString id;
+  if (InputElement().GetAttr(nsGkAtoms::list_, id)) {
+    Unlink();
+    RefPtr<nsAtom> idAtom = NS_AtomizeMainThread(id);
+    ResetWithID(InputElement(), idAtom);
+    AddObserverIfNeeded();
+  }
+  if (aRepaint) {
+    mOwningElementFrame->InvalidateFrame();
+  }
+}
+
+void ListMutationObserver::AddObserverIfNeeded() {
+  if (auto* list = get()) {
+    if (list->IsHTMLElement(nsGkAtoms::datalist)) {
+      list->AddMutationObserver(this);
+    }
+  }
+}
+
+void ListMutationObserver::RemoveObserverIfNeeded(dom::Element* aList) {
+  if (aList && aList->IsHTMLElement(nsGkAtoms::datalist)) {
+    aList->RemoveMutationObserver(this);
+  }
+}
+
+void ListMutationObserver::Detach() {
+  RemoveObserverIfNeeded();
+  Unlink();
+}
+
+dom::HTMLInputElement& ListMutationObserver::InputElement() const {
+  MOZ_ASSERT(mOwningElementFrame->GetContent()->IsHTMLElement(nsGkAtoms::input),
+             "bad cast");
+  return *static_cast<dom::HTMLInputElement*>(
+      mOwningElementFrame->GetContent());
+}
+
+void ListMutationObserver::AttributeChanged(dom::Element* aElement,
+                                            int32_t aNameSpaceID,
+                                            nsAtom* aAttribute,
+                                            int32_t aModType,
+                                            const nsAttrValue* aOldValue) {
+  if (aAttribute == nsGkAtoms::value && aNameSpaceID == kNameSpaceID_None &&
+      aElement->IsHTMLElement(nsGkAtoms::option)) {
+    mOwningElementFrame->InvalidateFrame();
+  }
+}
+
+void ListMutationObserver::CharacterDataChanged(
+    nsIContent* aContent, const CharacterDataChangeInfo& aInfo) {
+  mOwningElementFrame->InvalidateFrame();
+}
+
+void ListMutationObserver::ContentAppended(nsIContent* aFirstNewContent) {
+  mOwningElementFrame->InvalidateFrame();
+}
+
+void ListMutationObserver::ContentInserted(nsIContent* aChild) {
+  mOwningElementFrame->InvalidateFrame();
+}
+
+void ListMutationObserver::ContentRemoved(nsIContent* aChild,
+                                          nsIContent* aPreviousSibling) {
+  mOwningElementFrame->InvalidateFrame();
+}
+
+void ListMutationObserver::ElementChanged(dom::Element* aFrom,
+                                          dom::Element* aTo) {
+  IDTracker::ElementChanged(aFrom, aTo);
+  RemoveObserverIfNeeded(aFrom);
+  AddObserverIfNeeded();
+  mOwningElementFrame->InvalidateFrame();
+}
+
+}  // namespace mozilla

layout/forms/ListMutationObserver.h

@@ -0,0 +1,67 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+#ifndef mozilla_ListMutationObserver_h
+#define mozilla_ListMutationObserver_h
+
+#include "IDTracker.h"
+#include "nsStubMutationObserver.h"
+
+class nsIFrame;
+
+namespace mozilla {
+
+namespace dom {
+class HTMLInputElement;
+}  // namespace dom
+
+/**
+ * ListMutationObserver
+ * This class invalidates paint for the input element's frame when the content
+ * of its @list changes, or when the @list ID identifies a different element. It
+ * does *not* invalidate paint when the @list attribute itself changes.
+ */
+
+class ListMutationObserver final : public nsStubMutationObserver,
+                                   public dom::IDTracker {
+ public:
+  explicit ListMutationObserver(nsIFrame& aOwningElementFrame,
+                                bool aRepaint = false)
+      : mOwningElementFrame(&aOwningElementFrame) {
+    // We can skip invalidating paint if the frame is still being initialized.
+    Attach(aRepaint);
+  }
+
+  NS_DECL_ISUPPORTS
+
+  // We need to invalidate paint when the list or its options change.
+  NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
+  NS_DECL_NSIMUTATIONOBSERVER_CHARACTERDATACHANGED
+  NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
+  NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
+  NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
+
+  /**
+   * Triggered when the same @list ID identifies a different element than
+   * before.
+   */
+  void ElementChanged(dom::Element* aFrom, dom::Element* aTo) override;
+
+  void Attach(bool aRepaint = true);
+  void Detach();
+  void AddObserverIfNeeded();
+  void RemoveObserverIfNeeded(dom::Element* aList);
+  void RemoveObserverIfNeeded() { RemoveObserverIfNeeded(get()); }
+  dom::HTMLInputElement& InputElement() const;
+
+ private:
+  ~ListMutationObserver();
+
+  nsIFrame* mOwningElementFrame;
+};
+}  // namespace mozilla
+
+#endif  // mozilla_ListMutationObserver_h

layout/forms/moz.build

@@ -18,6 +18,7 @@ EXPORTS += [
 
 UNIFIED_SOURCES += [
     "HTMLSelectEventListener.cpp",
+    "ListMutationObserver.cpp",
     "nsButtonFrameRenderer.cpp",
     "nsCheckboxRadioFrame.cpp",
     "nsColorControlFrame.cpp",

layout/forms/nsRangeFrame.cpp

@@ -6,6 +6,7 @@
 
 #include "nsRangeFrame.h"
 
+#include "ListMutationObserver.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/PresShell.h"
 #include "mozilla/TouchEvents.h"
@@ -23,6 +24,7 @@
 #include "mozilla/dom/HTMLDataListElement.h"
 #include "mozilla/dom/HTMLInputElement.h"
 #include "mozilla/dom/HTMLOptionElement.h"
+#include "mozilla/dom/MutationEventBinding.h"
 #include "nsPresContext.h"
 #include "nsPresContextInlines.h"
 #include "nsNodeInfoManager.h"
@@ -50,6 +52,14 @@ nsIFrame* NS_NewRangeFrame(PresShell* aPresShell, ComputedStyle* aStyle) {
 nsRangeFrame::nsRangeFrame(ComputedStyle* aStyle, nsPresContext* aPresContext)
     : nsContainerFrame(aStyle, aPresContext, kClassID) {}
 
+void nsRangeFrame::Init(nsIContent* aContent, nsContainerFrame* aParent,
+                        nsIFrame* aPrevInFlow) {
+  nsContainerFrame::Init(aContent, aParent, aPrevInFlow);
+  if (InputElement().HasAttr(nsGkAtoms::list_)) {
+    mListMutationObserver = new ListMutationObserver(*this);
+  }
+}
+
 nsRangeFrame::~nsRangeFrame() = default;
 
 NS_IMPL_FRAMEARENA_HELPERS(nsRangeFrame)
@@ -65,6 +75,9 @@ void nsRangeFrame::DestroyFrom(nsIFrame* aDestructRoot,
                "nsRangeFrame should not have continuations; if it does we "
                "need to call RegUnregAccessKey only for the first.");
 
+  if (mListMutationObserver) {
+    mListMutationObserver->Detach();
+  }
   aPostDestroyData.AddAnonymousContent(mTrackDiv.forget());
   aPostDestroyData.AddAnonymousContent(mProgressDiv.forget());
   aPostDestroyData.AddAnonymousContent(mThumbDiv.forget());
@@ -636,6 +649,18 @@ nsresult nsRangeFrame::AttributeChanged(int32_t aNameSpaceID,
     } else if (aAttribute == nsGkAtoms::orient) {
       PresShell()->FrameNeedsReflow(this, IntrinsicDirty::None,
                                     NS_FRAME_IS_DIRTY);
+    } else if (aAttribute == nsGkAtoms::list_) {
+      const bool isRemoval = aModType == MutationEvent_Binding::REMOVAL;
+      if (mListMutationObserver) {
+        mListMutationObserver->Detach();
+        if (isRemoval) {
+          mListMutationObserver = nullptr;
+        } else {
+          mListMutationObserver->Attach();
+        }
+      } else if (!isRemoval) {
+        mListMutationObserver = new ListMutationObserver(*this, true);
+      }
     }
   }
 

layout/forms/nsRangeFrame.h

@@ -19,6 +19,7 @@
 class nsDisplayRangeFocusRing;
 
 namespace mozilla {
+class ListMutationObserver;
 class PresShell;
 namespace dom {
 class Event;
@@ -31,6 +32,9 @@ class nsRangeFrame final : public nsContainerFrame,
   friend nsIFrame* NS_NewRangeFrame(mozilla::PresShell* aPresShell,
                                     ComputedStyle* aStyle);
 
+  void Init(nsIContent* aContent, nsContainerFrame* aParent,
+            nsIFrame* aPrevInFlow) override;
+
   friend class nsDisplayRangeFocusRing;
 
   explicit nsRangeFrame(ComputedStyle* aStyle, nsPresContext* aPresContext);
@@ -195,6 +199,12 @@ class nsRangeFrame final : public nsContainerFrame,
    * @see nsRangeFrame::CreateAnonymousContent
    */
   nsCOMPtr<Element> mThumbDiv;
+
+  /**
+   * This mutation observer is used to invalidate paint when the @list changes,
+   * when a @list exists.
+   */
+  RefPtr<mozilla::ListMutationObserver> mListMutationObserver;
 };
 
 #endif

testing/web-platform/tests/html/semantics/forms/the-input-element/range-list-added-repaint.html

@@ -0,0 +1,19 @@
+<!doctype html>
+<html class=reftest-wait>
+<title>The range is repainted if a list ID is added</title>
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1805105">
+<link rel=author href="mailto:zach@zrhoffman.net" title="Zach Hoffman">
+<link rel=match href=range-tick-marks-05-ref.html>
+<script src=/common/reftest-wait.js></script>
+<input type=range step=3 value=1 min=-5 max=5>
+<datalist id=tickmarks>
+  <option value=4>
+  <option value=-2>
+</datalist>
+<script>
+  requestAnimationFrame(() =>
+    requestAnimationFrame(() => {
+      document.querySelector("input[type=range]").setAttribute("list", "tickmarks");
+      takeScreenshot();
+    }));
+</script>

testing/web-platform/tests/html/semantics/forms/the-input-element/range-list-change-repaint.html

@@ -0,0 +1,24 @@
+<!doctype html>
+<html class=reftest-wait>
+<title>The range is repainted if its list attribute changes</title>
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1805105">
+<link rel=author href="mailto:zach@zrhoffman.net" title="Zach Hoffman">
+<link rel=match href=range-tick-marks-05-ref.html>
+<script src=/common/reftest-wait.js></script>
+<input type=range step=3 value=1 min=-5 max=5 list=firstlist>
+<datalist id=firstlist>
+  <option value=1></option>
+  <option value=-5></option>
+</datalist>
+<datalist id=secondlist>
+  <option value=-2>
+  <option value=4>
+</datalist>
+<script>
+  requestAnimationFrame(() =>
+    requestAnimationFrame(() => {
+      const range = document.querySelector("input[type=range]");
+      range.setAttribute("list", "secondlist");
+      takeScreenshot();
+    }));
+</script>

testing/web-platform/tests/html/semantics/forms/the-input-element/range-list-duplicate-id-repaint.html

@@ -0,0 +1,26 @@
+<!doctype html>
+<html class=reftest-wait>
+<title>The range is repainted if the ID identifies a different list</title>
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1805105">
+<link rel=author href="mailto:zach@zrhoffman.net" title="Zach Hoffman">
+<link rel=match href=range-tick-marks-05-ref.html>
+<script src=/common/reftest-wait.js></script>
+<input type=range step=3 value=1 min=-5 max=5 list=firstlist>
+<datalist id=firstlist>
+  <option value=1></option>
+  <option value=-5></option>
+</datalist>
+<datalist id=secondlist>
+  <option value=4>
+  <option value=-2>
+</datalist>
+<script>
+  requestAnimationFrame(() =>
+    requestAnimationFrame(() => {
+      const firstList = document.querySelector("datalist#firstlist");
+      const secondList = document.querySelector("datalist#secondlist");
+      secondList.id = "firstlist";
+      firstList.parentNode.insertBefore(secondList, firstList);
+      takeScreenshot();
+    }));
+</script>

testing/web-platform/tests/html/semantics/forms/the-input-element/range-list-nonexistent.html

@@ -0,0 +1,20 @@
+<!doctype html>
+<html class=reftest-wait>
+<title>The range is repainted if the ID first identifies no list, then a list takes on that ID</title>
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1805105">
+<link rel=author href="mailto:zach@zrhoffman.net" title="Zach Hoffman">
+<link rel=match href=range-tick-marks-05-ref.html>
+<script src=/common/reftest-wait.js></script>
+<input type=range step=3 value=1 min=-5 max=5 list=nonexistentlist>
+<datalist>
+  <option value=4>
+  <option value=-2>
+</datalist>
+<script>
+  requestAnimationFrame(() =>
+    requestAnimationFrame(() => {
+      const dataListWithIDOfNonExistentList = document.querySelector("datalist");
+      dataListWithIDOfNonExistentList.id = "nonexistentlist";
+      takeScreenshot();
+    }));
+</script>

testing/web-platform/tests/html/semantics/forms/the-input-element/range-option-add-repaint.html

@@ -0,0 +1,21 @@
+<!doctype html>
+<html class=reftest-wait>
+<title>The range is repainted if an option is added to the range's list</title>
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1805105">
+<link rel=author href="mailto:zach@zrhoffman.net" title="Zach Hoffman">
+<link rel=match href=range-tick-marks-05-ref.html>
+<script src=/common/reftest-wait.js></script>
+<input type=range step=3 value=1 min=-5 max=5 list=tickmarks>
+<datalist id=tickmarks>
+  <option value=4></option>
+</datalist>
+<script>
+  requestAnimationFrame(() =>
+    requestAnimationFrame(() => {
+      const dataList = document.querySelector("datalist");
+      const toAdd = document.createElement("option");
+      toAdd.value = -2;
+      dataList.appendChild(toAdd);
+      takeScreenshot();
+    }));
+</script>

testing/web-platform/tests/html/semantics/forms/the-input-element/range-option-remove-repaint.html

@@ -0,0 +1,20 @@
+<!doctype html>
+<html class=reftest-wait>
+<title>The range is repainted if an option is removed from the range's list</title>
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1805105">
+<link rel=author href="mailto:zach@zrhoffman.net" title="Zach Hoffman">
+<link rel=match href=range-tick-marks-05-ref.html>
+<script src=/common/reftest-wait.js></script>
+<input type=range step=3 value=1 min=-5 max=5 list=tickmarks>
+<datalist id=tickmarks>
+  <option value=-2></option>
+  <option value=1 id=to-remove></option>
+  <option value=4></option>
+</datalist>
+<script>
+  requestAnimationFrame(() =>
+    requestAnimationFrame(() => {
+      document.querySelector("option#to-remove").remove();
+      takeScreenshot();
+    }));
+</script>

testing/web-platform/tests/html/semantics/forms/the-input-element/range-option-value-change-repaint.html

@@ -0,0 +1,20 @@
+<!doctype html>
+<html class=reftest-wait>
+<title>The range is repainted if the value of an option in the range's list changes</title>
+<link rel=help href="https://bugzilla.mozilla.org/show_bug.cgi?id=1805105">
+<link rel=author href="mailto:zach@zrhoffman.net" title="Zach Hoffman">
+<link rel=match href=range-tick-marks-05-ref.html>
+<script src=/common/reftest-wait.js></script>
+<input type=range step=3 value=1 min=-5 max=5 list=tickmarks>
+<datalist id=tickmarks>
+  <option value=-2></option>
+  <option value=1 id=to-change></option>
+</datalist>
+<script>
+  requestAnimationFrame(() =>
+    requestAnimationFrame(() => {
+      const toChange = document.querySelector("option#to-change");
+      toChange.value = 4;
+      takeScreenshot();
+    }));
+</script>