Bug 1801157 - Fix dynamic value change of label/description with acce…

…sskey. r=smaug While at it move GetAttributeChangeHint to relevant implementations. Differential Revision: https://phabricator.services.mozilla.com/D162335
dothq · Nov 17, 2022 · 2a31521 · 2a31521
1 parent cd6cf42
commit 2a31521
Show file tree

Hide file tree

Showing 8 changed files with 91 additions and 31 deletions.
diff --git a/dom/xul/XULButtonElement.cpp b/dom/xul/XULButtonElement.cpp
@@ -10,11 +10,22 @@
 #include "mozilla/TextEvents.h"
 #include "mozilla/dom/DocumentInlines.h"
 #include "mozilla/dom/MouseEventBinding.h"
+#include "nsChangeHint.h"
 #include "nsPresContext.h"
 #include "nsIDOMXULButtonElement.h"
 
 namespace mozilla::dom {
 
+nsChangeHint XULButtonElement::GetAttributeChangeHint(const nsAtom* aAttribute,
+                                                      int32_t aModType) const {
+  if (aAttribute == nsGkAtoms::type &&
+      IsAnyOfXULElements(nsGkAtoms::button, nsGkAtoms::toolbarbutton)) {
+    // type=menu switches to a menu frame.
+    return nsChangeHint_ReconstructFrame;
+  }
+  return nsXULElement::GetAttributeChangeHint(aAttribute, aModType);
+}
+
 nsresult XULButtonElement::PostHandleEvent(EventChainPostVisitor& aVisitor) {
   if (aVisitor.mEventStatus == nsEventStatus_eConsumeNoDefault) {
     return nsXULElement::PostHandleEvent(aVisitor);

diff --git a/dom/xul/XULButtonElement.h b/dom/xul/XULButtonElement.h
@@ -20,6 +20,10 @@ class XULButtonElement final : public nsXULElement {
   MOZ_CAN_RUN_SCRIPT_BOUNDARY bool MouseClicked(WidgetGUIEvent&);
   MOZ_CAN_RUN_SCRIPT nsresult PostHandleEvent(EventChainPostVisitor&) override;
 
+  nsChangeHint GetAttributeChangeHint(const nsAtom* aAttribute,
+                                      int32_t aModType) const override;
+
+
  private:
   void Blurred();
   bool mIsHandlingKeyEvent = false;

diff --git a/dom/xul/XULTextElement.cpp b/dom/xul/XULTextElement.cpp
@@ -7,14 +7,40 @@
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/ToJSValue.h"
 #include "nsCOMPtr.h"
+#include "nsChangeHint.h"
 #include "nsIContent.h"
 #include "nsPresContext.h"
 #include "nsIScrollableFrame.h"
+#include "mozilla/dom/MutationEventBinding.h"
 #include "mozilla/dom/XULTextElement.h"
 #include "mozilla/dom/XULTextElementBinding.h"
 
 namespace mozilla::dom {
 
+nsChangeHint XULTextElement::GetAttributeChangeHint(const nsAtom* aAttribute,
+                                                    int32_t aModType) const {
+  const bool reframe = [&] {
+    if (aAttribute == nsGkAtoms::value) {
+      // If we have an accesskey we need to recompute our -moz-label-content.
+      // Otherwise this is handled by either the attribute text node, or
+      // nsTextBoxFrame for crop="center".
+      return aModType == MutationEvent_Binding::ADDITION ||
+             aModType == MutationEvent_Binding::REMOVAL ||
+             HasAttr(nsGkAtoms::accesskey);
+    }
+    if (aAttribute == nsGkAtoms::crop || aAttribute == nsGkAtoms::accesskey) {
+      // value attr + crop="center" still uses nsTextBoxFrame. accesskey
+      // requires reframing as per the above comment.
+      return HasAttr(nsGkAtoms::value);
+    }
+    return false;
+  }();
+  if (reframe) {
+    return nsChangeHint_ReconstructFrame;
+  }
+  return nsXULElement::GetAttributeChangeHint(aAttribute, aModType);
+}
+
 JSObject* XULTextElement::WrapNode(JSContext* aCx,
                                    JS::Handle<JSObject*> aGivenProto) {
   return XULTextElement_Binding::Wrap(aCx, this, aGivenProto);

diff --git a/dom/xul/XULTextElement.h b/dom/xul/XULTextElement.h
@@ -33,6 +33,13 @@ class XULTextElement final : public nsXULElement {
     SetAttr(kNameSpaceID_None, nsGkAtoms::accesskey, aValue, true);
   }
 
+  nsChangeHint GetAttributeChangeHint(const nsAtom* aAttribute,
+                                      int32_t aModType) const override;
+
+  NS_IMPL_FROMNODE_HELPER(XULTextElement,
+                          IsAnyOfXULElements(nsGkAtoms::label,
+                                             nsGkAtoms::description));
+
  protected:
   virtual ~XULTextElement() = default;
   JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) final;

diff --git a/dom/xul/nsXULElement.cpp b/dom/xul/nsXULElement.cpp
@@ -1005,35 +1005,6 @@ nsresult nsXULElement::PreHandleEvent(EventChainVisitor& aVisitor) {
 //----------------------------------------------------------------------
 // Implementation methods
 
-nsChangeHint nsXULElement::GetAttributeChangeHint(const nsAtom* aAttribute,
-                                                  int32_t aModType) const {
-  if (IsAnyOfXULElements(nsGkAtoms::label, nsGkAtoms::description)) {
-    if (aAttribute == nsGkAtoms::value &&
-        (aModType == MutationEvent_Binding::REMOVAL ||
-         aModType == MutationEvent_Binding::ADDITION)) {
-      // Label and description dynamically morph between a normal block and a
-      // cropping single-line XUL text frame.  If the
-      // value attribute is being added or removed, then we need to
-      // return a hint of frame change.  (See bugzilla bug 95475 for
-      // details.)
-      return nsChangeHint_ReconstructFrame;
-    }
-    if ((aAttribute == nsGkAtoms::crop || aAttribute == nsGkAtoms::accesskey) &&
-        HasAttr(nsGkAtoms::value)) {
-      // They also can change based on crop="center", or accesskey.
-      return nsChangeHint_ReconstructFrame;
-    }
-  }
-
-  if (aAttribute == nsGkAtoms::type &&
-      IsAnyOfXULElements(nsGkAtoms::toolbarbutton, nsGkAtoms::button)) {
-    // type=menu switches from a button frame to a menu frame.
-    return nsChangeHint_ReconstructFrame;
-  }
-
-  return nsChangeHint(0);
-}
-
 NS_IMETHODIMP_(bool)
 nsXULElement::IsAttributeMapped(const nsAtom* aAttribute) const {
   return false;

diff --git a/dom/xul/nsXULElement.h b/dom/xul/nsXULElement.h
@@ -369,8 +369,6 @@ class nsXULElement : public nsStyledElement {
   virtual bool IsFocusableInternal(int32_t* aTabIndex,
                                    bool aWithMouse) override;
 
-  virtual nsChangeHint GetAttributeChangeHint(const nsAtom* aAttribute,
-                                              int32_t aModType) const override;
   NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
 
   virtual nsresult Clone(mozilla::dom::NodeInfo*,

diff --git a/layout/reftests/forms/textbox/accesskey-1-dyn.xhtml b/layout/reftests/forms/textbox/accesskey-1-dyn.xhtml
@@ -0,0 +1,42 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin/global.css"?>
+<window title="textbox access key tests (see bug 698185)"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+	xmlns:html="http://www.w3.org/1999/xhtml">
+  <html:style type="text/css">
+    hbox {
+      margin-top: 0px;
+      padding-top: 0px;
+      font-size: 36px;
+    }
+    label, input {
+      -moz-appearance: none;
+      background: inherit;
+      border: none 0px;
+    }
+    label {
+      margin-top: 0px;
+      padding-top: 0px;
+      margin-bottom: 0px;
+      padding-bottom: 0px;
+    }
+    input {
+      margin-top: 12px;
+      padding-top: 8px;
+      margin-bottom: 5px;
+      padding-bottom: 9px;
+    }
+  </html:style>
+
+  <hbox align="baseline">
+    <label value="foo" accesskey="s"/>
+    <input xmlns="http://www.w3.org/1999/xhtml" value="text"/>
+  </hbox>
+  <script>
+    onload = () {
+      let label = document.querySelector("label");
+      label.getBoundingClientRect();
+      label.value = "test";
+    };
+  </script>
+</window>
diff --git a/layout/reftests/forms/textbox/reftest.list b/layout/reftests/forms/textbox/reftest.list
@@ -1,6 +1,7 @@
 # access-key tests are no use on OS X because access keys are not indicated visually
 # no real XUL theme on Android so we just skip
 skip-if(cocoaWidget||Android) != chrome://reftest/content/forms/textbox/accesskey-1.xhtml chrome://reftest/content/forms/textbox/accesskey-1-notref.xhtml
+== chrome://reftest/content/forms/textbox/accesskey-1-dyn.xhtml chrome://reftest/content/forms/textbox/accesskey-1.xhtml
 fuzzy(0-1,0-3) skip-if(cocoaWidget||Android) == chrome://reftest/content/forms/textbox/accesskey-2.xhtml chrome://reftest/content/forms/textbox/accesskey-2-ref.xhtml
 skip-if(cocoaWidget||Android) == chrome://reftest/content/forms/textbox/accesskey-3.xhtml chrome://reftest/content/forms/textbox/accesskey-3-ref.xhtml
 skip-if(cocoaWidget||Android) != chrome://reftest/content/forms/textbox/accesskey-3.xhtml chrome://reftest/content/forms/textbox/accesskey-3-notref.xhtml