Bug 981248 - Rewrite <input type=number> to avoid an anonymous input.…

… r=masayuki,surkov,jwatt,ntim,jfkthame,smaug Instead, subclass nsTextControlFrame. This simplifies the code and avoids correctness issues. I kept the localization functionality though it is not spec compliant. But I filed a bug to remove it in a followup. Differential Revision: https://phabricator.services.mozilla.com/D57193
iaxat · Jan 14, 2020 · 8ec45d6 · 8ec45d6
1 parent 118d16f
commit 8ec45d6
Show file tree

Hide file tree

Showing 45 changed files with 367 additions and 1,169 deletions.
diff --git a/accessible/base/RoleMap.h b/accessible/base/RoleMap.h
@@ -492,7 +492,7 @@ ROLE(SPINBUTTON,
      NSAccessibilityIncrementorRole,  //Subroles: Increment/Decrement.
      ROLE_SYSTEM_SPINBUTTON,
      ROLE_SYSTEM_SPINBUTTON,
-     java::SessionAccessibility::CLASSNAME_VIEW, // A composite widget
+     java::SessionAccessibility::CLASSNAME_EDITTEXT,
      eNameFromValueRule)
 
 ROLE(DIAGRAM,

diff --git a/accessible/html/HTMLFormControlAccessible.cpp b/accessible/html/HTMLFormControlAccessible.cpp
@@ -466,7 +466,7 @@ Accessible* HTMLFileInputAccessible::CurrentItem() const {
 role HTMLSpinnerAccessible::NativeRole() const { return roles::SPINBUTTON; }
 
 void HTMLSpinnerAccessible::Value(nsString& aValue) const {
-  AccessibleWrap::Value(aValue);
+  HTMLTextFieldAccessible::Value(aValue);
   if (!aValue.IsEmpty()) return;
 
   // Pass NonSystem as the caller type, to be safe.  We don't expect to have a
@@ -475,28 +475,28 @@ void HTMLSpinnerAccessible::Value(nsString& aValue) const {
 }
 
 double HTMLSpinnerAccessible::MaxValue() const {
-  double value = AccessibleWrap::MaxValue();
+  double value = HTMLTextFieldAccessible::MaxValue();
   if (!IsNaN(value)) return value;
 
   return HTMLInputElement::FromNode(mContent)->GetMaximum().toDouble();
 }
 
 double HTMLSpinnerAccessible::MinValue() const {
-  double value = AccessibleWrap::MinValue();
+  double value = HTMLTextFieldAccessible::MinValue();
   if (!IsNaN(value)) return value;
 
   return HTMLInputElement::FromNode(mContent)->GetMinimum().toDouble();
 }
 
 double HTMLSpinnerAccessible::Step() const {
-  double value = AccessibleWrap::Step();
+  double value = HTMLTextFieldAccessible::Step();
   if (!IsNaN(value)) return value;
 
   return HTMLInputElement::FromNode(mContent)->GetStep().toDouble();
 }
 
 double HTMLSpinnerAccessible::CurValue() const {
-  double value = AccessibleWrap::CurValue();
+  double value = HTMLTextFieldAccessible::CurValue();
   if (!IsNaN(value)) return value;
 
   return HTMLInputElement::FromNode(mContent)->GetValueAsDecimal().toDouble();

diff --git a/accessible/html/HTMLFormControlAccessible.h b/accessible/html/HTMLFormControlAccessible.h
@@ -63,7 +63,7 @@ class HTMLButtonAccessible : public HyperTextAccessibleWrap {
  * Accessible for HTML input@type="text", input@type="password", textarea and
  * other HTML text controls.
  */
-class HTMLTextFieldAccessible final : public HyperTextAccessibleWrap {
+class HTMLTextFieldAccessible : public HyperTextAccessibleWrap {
  public:
   enum { eAction_Click = 0 };
 
@@ -99,8 +99,7 @@ class HTMLTextFieldAccessible final : public HyperTextAccessibleWrap {
   virtual ENameValueFlag NativeName(nsString& aName) const override;
 
   /**
-   * Return a widget element this input is part of, for example, search-textbox
-   * or HTML:input@type="number".
+   * Return a widget element this input is part of, for example, search-textbox.
    *
    * FIXME: This should probably be renamed.
    */
@@ -129,10 +128,10 @@ class HTMLFileInputAccessible : public HyperTextAccessibleWrap {
 /**
  * Used for HTML input@type="number".
  */
-class HTMLSpinnerAccessible : public AccessibleWrap {
+class HTMLSpinnerAccessible final : public HTMLTextFieldAccessible {
  public:
   HTMLSpinnerAccessible(nsIContent* aContent, DocAccessible* aDoc)
-      : AccessibleWrap(aContent, aDoc) {
+      : HTMLTextFieldAccessible(aContent, aDoc) {
     mStateFlags |= eHasNumericValue;
   }
 

diff --git a/accessible/tests/mochitest/attributes/test_obj.html b/accessible/tests/mochitest/attributes/test_obj.html
@@ -124,7 +124,7 @@
       testAttrs("search", {"text-input-type": "search"}, true);
       testAttrs("tel", {"text-input-type": "tel"}, true);
       testAttrs("url", {"text-input-type": "url"}, true);
-      testAttrs(getAccessible("number").firstChild, {"text-input-type": "number"}, true);
+      testAttrs("number", {"text-input-type": "number"}, true);
 
       // ARIA
       testAttrs("searchbox", {"text-input-type": "search"}, true);

diff --git a/accessible/tests/mochitest/elm/test_HTMLSpec.html b/accessible/tests/mochitest/elm/test_HTMLSpec.html
@@ -712,15 +712,7 @@
         role: ROLE_SPINBUTTON,
         interfaces: [ nsIAccessibleValue ],
         children: [
-          {
-            role: ROLE_ENTRY,
-            extraStates: EXT_STATE_EDITABLE | EXT_STATE_SINGLE_LINE,
-            actions: "activate",
-            interfaces: [ nsIAccessibleText, nsIAccessibleEditableText ],
-            children: [
-              { role: ROLE_TEXT_LEAF },
-            ],
-          },
+          { role: ROLE_TEXT_LEAF },
           {
             role: ROLE_PUSHBUTTON,
             actions: "press",

diff --git a/accessible/tests/mochitest/tree/test_formctrl.html b/accessible/tests/mochitest/tree/test_formctrl.html
@@ -64,7 +64,6 @@
       // input@type="number"
       accTree =
         { SPINBUTTON: [
-          { ENTRY: [ ] },
           { PUSHBUTTON: [ ] },
           { PUSHBUTTON: [ ] },
         ] };

diff --git a/browser/components/preferences/connection.xhtml b/browser/components/preferences/connection.xhtml
@@ -59,7 +59,7 @@
           <html:input id="networkProxyHTTP" type="text" style="-moz-box-flex: 1;"
                       preference="network.proxy.http"/>
           <label data-l10n-id="connection-proxy-http-port" control="networkProxyHTTP_Port" />
-          <html:input id="networkProxyHTTP_Port" class="proxy-port-input input-number-mozbox" hidespinbuttons="true" type="number" min="0" max="65535"
+          <html:input id="networkProxyHTTP_Port" class="proxy-port-input" hidespinbuttons="true" type="number" min="0" max="65535"
                       preference="network.proxy.http_port"/>
         </hbox>
         <hbox/>
@@ -73,7 +73,7 @@
         <hbox align="center">
           <html:input id="networkProxySSL" type="text" style="-moz-box-flex: 1;" preference="network.proxy.ssl"/>
           <label data-l10n-id="connection-proxy-ssl-port" control="networkProxySSL_Port" />
-          <html:input id="networkProxySSL_Port" class="proxy-port-input input-number-mozbox" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
+          <html:input id="networkProxySSL_Port" class="proxy-port-input" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
                       preference="network.proxy.ssl_port"/>
         </hbox>
         <hbox pack="end">
@@ -82,7 +82,7 @@
         <hbox align="center">
           <html:input id="networkProxyFTP" type="text" style="-moz-box-flex: 1;" preference="network.proxy.ftp"/>
           <label data-l10n-id="connection-proxy-ftp-port" control="networkProxyFTP_Port"/>
-          <html:input id="networkProxyFTP_Port" class="proxy-port-input input-number-mozbox" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
+          <html:input id="networkProxyFTP_Port" class="proxy-port-input" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
                       preference="network.proxy.ftp_port"/>
         </hbox>
         <separator class="thin"/>
@@ -92,7 +92,7 @@
         <hbox align="center">
           <html:input id="networkProxySOCKS" type="text" style="-moz-box-flex: 1;" preference="network.proxy.socks"/>
           <label data-l10n-id="connection-proxy-socks-port" control="networkProxySOCKS_Port"/>
-          <html:input id="networkProxySOCKS_Port" class="proxy-port-input input-number-mozbox" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
+          <html:input id="networkProxySOCKS_Port" class="proxy-port-input" hidespinbuttons="true" type="number" min="0" max="65535" size="5"
                       preference="network.proxy.socks_port"/>
         </hbox>
         <spacer/>

diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp
@@ -60,7 +60,6 @@
 #include "nsITextControlFrame.h"
 #include "nsCommandManager.h"
 #include "nsCommandParams.h"
-#include "nsNumberControlFrame.h"
 #include "nsUnicharUtils.h"
 #include "nsContentList.h"
 #include "nsCSSPseudoElements.h"
@@ -12344,9 +12343,7 @@ already_AddRefed<nsDOMCaretPosition> Document::CaretPositionFromPoint(
     nsIContent* nonanon = node->FindFirstNonChromeOnlyAccessContent();
     HTMLTextAreaElement* textArea = HTMLTextAreaElement::FromNode(nonanon);
     nsITextControlFrame* textFrame = do_QueryFrame(nonanon->GetPrimaryFrame());
-    nsNumberControlFrame* numberFrame =
-        do_QueryFrame(nonanon->GetPrimaryFrame());
-    if (textFrame || numberFrame) {
+    if (textFrame) {
       // If the anonymous content node has a child, then we need to make sure
       // that we get the appropriate child, as otherwise the offset may not be
       // correct when we construct a range for it.

diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp
@@ -37,7 +37,6 @@
 #include "BrowserChild.h"
 #include "nsFrameLoader.h"
 #include "nsHTMLDocument.h"
-#include "nsNumberControlFrame.h"
 #include "nsNetUtil.h"
 #include "nsRange.h"
 
@@ -316,24 +315,6 @@ Element* nsFocusManager::GetFocusedDescendant(
 
 // static
 Element* nsFocusManager::GetRedirectedFocus(nsIContent* aContent) {
-  // For input number, redirect focus to our anonymous text control.
-  if (aContent->IsHTMLElement(nsGkAtoms::input)) {
-    bool typeIsNumber =
-        static_cast<dom::HTMLInputElement*>(aContent)->ControlType() ==
-        NS_FORM_INPUT_NUMBER;
-
-    if (typeIsNumber) {
-      nsNumberControlFrame* numberControlFrame =
-          do_QueryFrame(aContent->GetPrimaryFrame());
-
-      if (numberControlFrame) {
-        HTMLInputElement* textControl =
-            numberControlFrame->GetAnonTextControl();
-        return textControl;
-      }
-    }
-  }
-
 #ifdef MOZ_XUL
   if (aContent->IsXULElement()) {
     nsCOMPtr<nsIDOMXULMenuListElement> menulist =
@@ -1479,8 +1460,7 @@ Element* nsFocusManager::FlushAndCheckIfFocusable(Element* aElement,
 
   // this is a special case for some XUL elements or input number, where an
   // anonymous child is actually focusable and not the element itself.
-  RefPtr<Element> redirectedFocus = GetRedirectedFocus(aElement);
-  if (redirectedFocus) {
+  if (RefPtr<Element> redirectedFocus = GetRedirectedFocus(aElement)) {
     return FlushAndCheckIfFocusable(redirectedFocus, aFlags);
   }
 

diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp
@@ -1580,7 +1580,7 @@ void EventStateManager::FireContextClick() {
 
       if (formCtrl) {
         allowedToDispatch =
-            formCtrl->IsTextOrNumberControl(/*aExcludePassword*/ false) ||
+            formCtrl->IsTextControl(/*aExcludePassword*/ false) ||
             formCtrl->ControlType() == NS_FORM_INPUT_FILE;
       } else if (mGestureDownContent->IsAnyOfHTMLElements(
                      nsGkAtoms::embed, nsGkAtoms::object, nsGkAtoms::label)) {

diff --git a/dom/events/IMEStateManager.cpp b/dom/events/IMEStateManager.cpp
@@ -1122,7 +1122,7 @@ void IMEStateManager::SetInputContextForChildProcess(
   SetInputContext(widget, aInputContext, aAction);
 }
 
-static bool IsNextFocusableElementTextOrNumberControl(Element* aInputContent) {
+static bool IsNextFocusableElementTextControl(Element* aInputContent) {
   nsFocusManager* fm = nsFocusManager::GetFocusManager();
   if (!fm) {
     return false;
@@ -1136,7 +1136,7 @@ static bool IsNextFocusableElementTextOrNumberControl(Element* aInputContent) {
   }
   nextContent = nextContent->FindFirstNonChromeOnlyAccessContent();
   nsCOMPtr<nsIFormControl> nextControl = do_QueryInterface(nextContent);
-  if (!nextControl || !nextControl->IsTextOrNumberControl(false)) {
+  if (!nextControl || !nextControl->IsTextControl(false)) {
     return false;
   }
 
@@ -1217,8 +1217,7 @@ static void GetActionHint(nsIContent& aContent, nsAString& aActionHint) {
     if (!isLastElement && formElement) {
       // If next tabbable content in form is text control, hint should be "next"
       // even there is submit in form.
-      if (IsNextFocusableElementTextOrNumberControl(
-              inputContent->AsElement())) {
+      if (IsNextFocusableElementTextControl(inputContent->AsElement())) {
         // This is focusable text control
         // XXX What good hint for read only field?
         aActionHint.AssignLiteral("next");
@@ -1274,22 +1273,8 @@ void IMEStateManager::SetIMEState(const IMEState& aState,
   if (aContent &&
       aContent->IsAnyOfHTMLElements(nsGkAtoms::input, nsGkAtoms::textarea)) {
     if (!aContent->IsHTMLElement(nsGkAtoms::textarea)) {
-      // <input type=number> has an anonymous <input type=text> descendant
-      // that gets focus whenever anyone tries to focus the number control. We
-      // need to check if aContent is one of those anonymous text controls and,
-      // if so, use the number control instead:
-      Element* element = aContent->AsElement();
-      HTMLInputElement* inputElement =
-          HTMLInputElement::FromNodeOrNull(aContent);
-      if (inputElement) {
-        HTMLInputElement* ownerNumberControl =
-            inputElement->GetOwnerNumberControl();
-        if (ownerNumberControl) {
-          element = ownerNumberControl;  // an <input type=number>
-        }
-      }
-      element->GetAttr(kNameSpaceID_None, nsGkAtoms::type,
-                       context.mHTMLInputType);
+      aContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::type,
+                                     context.mHTMLInputType);
     } else {
       context.mHTMLInputType.Assign(nsGkAtoms::textarea->GetUTF16String());
     }

diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp
@@ -1754,7 +1754,7 @@ bool HTMLFormElement::ImplicitSubmissionIsDisabled() const {
   uint32_t numDisablingControlsFound = 0;
   uint32_t length = mControls->mElements.Length();
   for (uint32_t i = 0; i < length && numDisablingControlsFound < 2; ++i) {
-    if (mControls->mElements[i]->IsSingleLineTextOrNumberControl(false)) {
+    if (mControls->mElements[i]->IsSingleLineTextControl(false)) {
       numDisablingControlsFound++;
     }
   }
@@ -1767,7 +1767,7 @@ bool HTMLFormElement::IsLastActiveElement(
 
   for (auto* element : Reversed(mControls->mElements)) {
     // XXX How about date/time control?
-    if (element->IsTextOrNumberControl(false) && !element->IsDisabled()) {
+    if (element->IsTextControl(false) && !element->IsDisabled()) {
       return element == aControl;
     }
   }