Bug 606528. Don't map the type attribute for inputs. r=emilio

The change to test_bug558726.html is because we always serialize mapped attributes after non-mapped ones. So this change is actually improving our serialization in that test.
2018-07-06 11:43:14 -07:00 · 2018-07-06 11:43:14 -07:00 · ce664a172d
--- a/dom/base/nsAttrAndChildArray.cpp
+++ b/dom/base/nsAttrAndChildArray.cpp
@ -628,6 +628,19 @@ nsAttrAndChildArray::DoSetMappedAttrStyleSheet(nsHTMLStyleSheet* aSheet)
  return MakeMappedUnique(mapped);
 }

+nsresult
+nsAttrAndChildArray::DoUpdateMappedAttrRuleMapper(nsMappedAttributeElement& aElement)
+{
+  MOZ_ASSERT(mImpl && mImpl->mMappedAttrs, "Should have mapped attrs here!");
+
+  // First two args don't matter if the assert holds.
+  RefPtr<nsMappedAttributes> mapped =
+    GetModifiableMapped(nullptr, nullptr, false);
+
+  mapped->SetRuleMapper(aElement.GetAttributeMappingFunction());
+
+  return MakeMappedUnique(mapped);
+}

 void
 nsAttrAndChildArray::Compact()
--- a/dom/base/nsAttrAndChildArray.h
+++ b/dom/base/nsAttrAndChildArray.h
@ -126,6 +126,17 @@ public:
    return DoSetMappedAttrStyleSheet(aSheet);
  }

+  // Update the rule mapping function on our mapped attributes, if we have any.
+  // We take a nsMappedAttributeElement, not a nsMapRuleToAttributesFunc,
+  // because the latter is defined in a header we can't include here.
+  nsresult UpdateMappedAttrRuleMapper(nsMappedAttributeElement& aElement)
+  {
+    if (!mImpl || !mImpl->mMappedAttrs) {
+      return NS_OK;
+    }
+    return DoUpdateMappedAttrRuleMapper(aElement);
+  }
+
  void Compact();

  bool CanFitMoreAttrs() const
@ -224,6 +235,11 @@ private:
   */
  nsresult DoSetMappedAttrStyleSheet(nsHTMLStyleSheet* aSheet);

+  /**
+   * Guts of UpdateMappedAttrRuleMapper for the case  when we have mapped attrs.
+   */
+  nsresult DoUpdateMappedAttrRuleMapper(nsMappedAttributeElement& aElement);
+
  struct InternalAttr
  {
    nsAttrName mName;
--- a/dom/base/nsMappedAttributes.h
+++ b/dom/base/nsMappedAttributes.h
@ -56,6 +56,11 @@ public:
    return mSheet;
  }

+  void SetRuleMapper(nsMapRuleToAttributesFunc aRuleMapper)
+  {
+    mRuleMapper = aRuleMapper;
+  }
+
  const nsAttrName* NameAt(uint32_t aPos) const
  {
    NS_ASSERTION(aPos < mAttrCount, "out-of-bounds");
--- a/dom/base/test/test_bug558726.html
+++ b/dom/base/test/test_bug558726.html
@ -25,7 +25,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=558726
 /** Test for Bug 558726 **/

 function runTest() {
-  is(document.getElementById('test_558726_1').innerHTML, '<select name="selecttest"><option value="0">item 1</option></select>x<input name="test" value="test" type="button">','test_558726_1.innerHTML')
+  is(document.getElementById('test_558726_1').innerHTML, '<select name="selecttest"><option value="0">item 1</option></select>x<input type="button" name="test" value="test">','test_558726_1.innerHTML')
  is(document.getElementById('test_558726_2').innerHTML, 'y<input><i style="-moz-user-select: none;">z</i>','test_558726_2.innerHTML')
  SimpleTest.finish();
 }
--- a/dom/html/HTMLInputElement.cpp
+++ b/dom/html/HTMLInputElement.cpp
@ -4832,18 +4832,26 @@ HTMLInputElement::HandleTypeChange(uint8_t aNewType, bool aNotify)
    // We're no longer an image input.  Cancel our image requests, if we have
    // any.
    CancelImageRequests(aNotify);
-  } else if (aNotify && mType == NS_FORM_INPUT_IMAGE) {
-    // We just got switched to be an image input; we should see
-    // whether we have an image to load;
-    nsAutoString src;
-    if (GetAttr(kNameSpaceID_None, nsGkAtoms::src, src)) {
-      // Mark channel as urgent-start before load image if the image load is
-      // initaiated by a user interaction.
-      mUseUrgentStartForChannel = EventStateManager::IsHandlingUserInput();

-      LoadImage(src, false, aNotify, eImageLoadType_Normal,
-                mSrcTriggeringPrincipal);
+    // And we should update our mapped attribute mapping function.
+    mAttrsAndChildren.UpdateMappedAttrRuleMapper(*this);
+  } else if (mType == NS_FORM_INPUT_IMAGE) {
+    if (aNotify) {
+      // We just got switched to be an image input; we should see
+      // whether we have an image to load;
+      nsAutoString src;
+      if (GetAttr(kNameSpaceID_None, nsGkAtoms::src, src)) {
+        // Mark channel as urgent-start before load image if the image load is
+        // initaiated by a user interaction.
+        mUseUrgentStartForChannel = EventStateManager::IsHandlingUserInput();
+
+        LoadImage(src, false, aNotify, eImageLoadType_Normal,
+                  mSrcTriggeringPrincipal);
+      }
    }
+
+    // And we should update our mapped attribute mapping function.
+    mAttrsAndChildren.UpdateMappedAttrRuleMapper(*this);
  }

  if (mType == NS_FORM_INPUT_PASSWORD && IsInComposedDoc()) {
@ -5595,18 +5603,14 @@ HTMLInputElement::ParseAttribute(int32_t aNamespaceID,
 }

 void
-HTMLInputElement::MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
-                                        MappedDeclarations& aDecls)
+HTMLInputElement::ImageInputMapAttributesIntoRule(const nsMappedAttributes* aAttributes,
+                                                  MappedDeclarations& aDecls)
 {
-  const nsAttrValue* value = aAttributes->GetAttr(nsGkAtoms::type);
-  if (value && value->Type() == nsAttrValue::eEnum &&
-      value->GetEnumValue() == NS_FORM_INPUT_IMAGE) {
-    nsGenericHTMLFormElementWithState::MapImageBorderAttributeInto(aAttributes, aDecls);
-    nsGenericHTMLFormElementWithState::MapImageMarginAttributeInto(aAttributes, aDecls);
-    nsGenericHTMLFormElementWithState::MapImageSizeAttributesInto(aAttributes, aDecls);
-    // Images treat align as "float"
-    nsGenericHTMLFormElementWithState::MapImageAlignAttributeInto(aAttributes, aDecls);
-  }
+  nsGenericHTMLFormElementWithState::MapImageBorderAttributeInto(aAttributes, aDecls);
+  nsGenericHTMLFormElementWithState::MapImageMarginAttributeInto(aAttributes, aDecls);
+  nsGenericHTMLFormElementWithState::MapImageSizeAttributesInto(aAttributes, aDecls);
+  // Images treat align as "float"
+  nsGenericHTMLFormElementWithState::MapImageAlignAttributeInto(aAttributes, aDecls);

  nsGenericHTMLFormElementWithState::MapCommonAttributesInto(aAttributes, aDecls);
 }
@ -5645,7 +5649,6 @@ HTMLInputElement::IsAttributeMapped(const nsAtom* aAttribute) const
 {
  static const MappedAttributeEntry attributes[] = {
    { &nsGkAtoms::align },
-    { &nsGkAtoms::type },
    { nullptr },
  };

@ -5662,7 +5665,15 @@ HTMLInputElement::IsAttributeMapped(const nsAtom* aAttribute) const
 nsMapRuleToAttributesFunc
 HTMLInputElement::GetAttributeMappingFunction() const
 {
-  return &MapAttributesIntoRule;
+  // GetAttributeChangeHint guarantees that changes to mType will trigger a
+  // reframe, and we update the mapping function in our mapped attrs when our
+  // type changes, so it's safe to condition our attribute mapping function on
+  // mType.
+  if (mType == NS_FORM_INPUT_IMAGE) {
+    return &ImageInputMapAttributesIntoRule;
+  }
+
+  return &MapCommonAttributesInto;
 }


--- a/dom/html/HTMLInputElement.h
+++ b/dom/html/HTMLInputElement.h
@ -1666,8 +1666,8 @@ protected:
  bool                     mHasPatternAttribute : 1;

 private:
-  static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
-                                    MappedDeclarations&);
+  static void ImageInputMapAttributesIntoRule(const nsMappedAttributes* aAttributes,
+                                              MappedDeclarations&);

  /**
   * Returns true if this input's type will fire a DOM "change" event when it
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@ -181867,6 +181867,42 @@
     {}
    ]
   ],
+   "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-1.html": [
+    [
+     "/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-1.html",
+     [
+      [
+       "/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
+   "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-2.html": [
+    [
+     "/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-2.html",
+     [
+      [
+       "/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
+   "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1.html": [
+    [
+     "/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1.html",
+     [
+      [
+       "/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1-ref.html",
+       "=="
+      ]
+     ],
+     {}
+    ]
+   ],
   "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/object_border_perc.xhtml": [
    [
     "/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/object_border_perc.xhtml",
@ -283737,6 +283773,16 @@
     {}
    ]
   ],
+   "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-ref.html": [
+    [
+     {}
+    ]
+   ],
+   "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1-ref.html": [
+    [
+     {}
+    ]
+   ],
   "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/object_border-ref.xhtml": [
    [
     {}
@ -436380,11 +436426,11 @@
   "testharness"
  ],
  "content-security-policy/securitypolicyviolation/blockeduri-eval.html": [
-   "01d4ce834d175d13eb0d9c80bbe4a7be614d687f",
+   "26a5ce6bed6fba88412ffbd7a0aa0c04c4b9c86d",
   "testharness"
  ],
  "content-security-policy/securitypolicyviolation/blockeduri-inline.html": [
-   "8e7326101e28ec65c6c834f7711b261917f93218",
+   "ff8d1c26701d15164bcb549d42937825e1f0edd0",
   "testharness"
  ],
  "content-security-policy/securitypolicyviolation/idl.html": [
@ -436456,7 +436502,7 @@
   "support"
  ],
  "content-security-policy/securitypolicyviolation/targeting.html": [
-   "36ec8dd9ef0bd1be3615913015d857aa1a7c9e97",
+   "44a9c8d566dd2532d09972df3b5745db671ed8dc",
   "testharness"
  ],
  "content-security-policy/securitypolicyviolation/upgrade-insecure-requests-reporting.https.html": [
@ -579195,6 +579241,26 @@
   "c1cff7b05cc400195328292521ecf76840d60540",
   "reftest"
  ],
+  "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-1.html": [
+   "0c229767f50480bc044975494ffbc5c30b9a38ec",
+   "reftest"
+  ],
+  "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-2.html": [
+   "691b374d8852aaaa5459a125989ee25e46632224",
+   "reftest"
+  ],
+  "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-ref.html": [
+   "7e62d0a23322cf52a49ab203bbd2ce3224cb2cc1",
+   "support"
+  ],
+  "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1-ref.html": [
+   "db5ba23b7ff29945bb5c7ba71a9c24567c8a5446",
+   "support"
+  ],
+  "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1.html": [
+   "32df51d982607e285e34199ce3f1f90ad208deb5",
+   "reftest"
+  ],
  "html/rendering/replaced-elements/attributes-for-embedded-content-and-images/object_border-ref.xhtml": [
   "48efda6cb8f06cc70c71c436cecf6f3e4fea6aec",
   "support"
--- a/testing/web-platform/meta/dom/collections/namednodemap-supported-property-names.html.ini
+++ b/testing/web-platform/meta/dom/collections/namednodemap-supported-property-names.html.ini
@ -1,9 +0,0 @@
-[namednodemap-supported-property-names.html]
-  [Object.getOwnPropertyNames on NamedNodeMap of input]
-    expected: FAIL
-    bug: 1303629
-
-  [Object.getOwnPropertyNames on NamedNodeMap after attribute removal]
-    expected: FAIL
-    bug: 1303629
-
--- a/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-1.html
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-1.html
@ -0,0 +1,3 @@
+<!DOCTYPE html>
+<link rel="match" href="input-align-right-ref.html">
+<input type="image" align="right">
--- a/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-2.html
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-2.html
@ -0,0 +1,3 @@
+<!DOCTYPE html>
+<link rel="match" href="input-align-right-ref.html">
+<input align="right" type="image">
--- a/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-ref.html
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-align-right-ref.html
@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<input type="image" style="float: right">
--- a/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1-ref.html
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1-ref.html
@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<input align="right">
--- a/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1.html
+++ b/testing/web-platform/tests/html/rendering/replaced-elements/attributes-for-embedded-content-and-images/input-type-change-from-image-1.html
@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<link rel=match href="input-type-change-from-image-1-ref.html">
+<input type="image" align="right">
+<script>
+  onload = function() {
+    var i = document.querySelector("input");
+    window.rect = i.getBoundingClientRect();
+    i.type = "text";
+  }
+</script>