Bug 1562257 part 5. Fix style mapping of border attribues to more closely match the spec. r=mccr8
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 28 Jun 2019 23:55:38 +0000
changeset 480719 b8938b6afa9c4b498ec016d1b3d50be0e50684c0
parent 480718 bd2a2e91e9eefc6f27621620e11b93ffd8eca2e6
child 480720 4c18b27b133c5052954190d74c4416904f38c0f2
push id113563
push userapavel@mozilla.com
push dateSun, 30 Jun 2019 10:04:30 +0000
treeherdermozilla-inbound@b8938b6afa9c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1562257
milestone69.0a1
first release with
nightly linux64
b8938b6afa9c / 69.0a1 / 20190630095236 / files
nightly mac
b8938b6afa9c / 69.0a1 / 20190630095236 / files
nightly win32
b8938b6afa9c / 69.0a1 / 20190630095236 / files
nightly win64
b8938b6afa9c / 69.0a1 / 20190630095236 / files
nightly linux32
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1562257 part 5. Fix style mapping of border attribues to more closely match the spec. r=mccr8 Per spec, "border" is parsed as a non-negative integer, only mapped if nonzero (though this is not observably different from mapping even if 0, except if user or UA stylesheets style the border), and supported on img, object, <input type="image">, but NOT embed, iframe, or marquee. This matches the Chrome and Safari behavior, as far as I can tell. The substantive change here is that we are removing mapping for the <embed border> case. Differential Revision: https://phabricator.services.mozilla.com/D36376
dom/html/HTMLEmbedElement.cpp
dom/html/nsGenericHTMLElement.cpp
testing/web-platform/tests/html/rendering/pixel-length-attributes.html
testing/web-platform/tests/html/rendering/unmapped-attributes.html
--- a/dom/html/HTMLEmbedElement.cpp
+++ b/dom/html/HTMLEmbedElement.cpp
@@ -182,28 +182,28 @@ int32_t HTMLEmbedElement::TabIndexDefaul
 bool HTMLEmbedElement::ParseAttribute(int32_t aNamespaceID, nsAtom* aAttribute,
                                       const nsAString& aValue,
                                       nsIPrincipal* aMaybeScriptedPrincipal,
                                       nsAttrValue& aResult) {
   if (aNamespaceID == kNameSpaceID_None) {
     if (aAttribute == nsGkAtoms::align) {
       return ParseAlignValue(aValue, aResult);
     }
-    if (ParseImageAttribute(aAttribute, aValue, aResult)) {
-      return true;
+    if (aAttribute == nsGkAtoms::width || aAttribute == nsGkAtoms::height ||
+        aAttribute == nsGkAtoms::hspace || aAttribute == nsGkAtoms::vspace) {
+      return aResult.ParseHTMLDimension(aValue);
     }
   }
 
   return nsGenericHTMLElement::ParseAttribute(aNamespaceID, aAttribute, aValue,
                                               aMaybeScriptedPrincipal, aResult);
 }
 
 static void MapAttributesIntoRuleBase(const nsMappedAttributes* aAttributes,
                                       MappedDeclarations& aDecls) {
-  nsGenericHTMLElement::MapImageBorderAttributeInto(aAttributes, aDecls);
   nsGenericHTMLElement::MapImageMarginAttributeInto(aAttributes, aDecls);
   nsGenericHTMLElement::MapImageSizeAttributesInto(aAttributes, aDecls);
   nsGenericHTMLElement::MapImageAlignAttributeInto(aAttributes, aDecls);
 }
 
 static void MapAttributesIntoRuleExceptHidden(
     const nsMappedAttributes* aAttributes, MappedDeclarations& aDecls) {
   MapAttributesIntoRuleBase(aAttributes, aDecls);
--- a/dom/html/nsGenericHTMLElement.cpp
+++ b/dom/html/nsGenericHTMLElement.cpp
@@ -1038,17 +1038,17 @@ bool nsGenericHTMLElement::ParseDivAlign
 bool nsGenericHTMLElement::ParseImageAttribute(nsAtom* aAttribute,
                                                const nsAString& aString,
                                                nsAttrValue& aResult) {
   if (aAttribute == nsGkAtoms::width || aAttribute == nsGkAtoms::height ||
       aAttribute == nsGkAtoms::hspace || aAttribute == nsGkAtoms::vspace) {
     return aResult.ParseHTMLDimension(aString);
   }
   if (aAttribute == nsGkAtoms::border) {
-    return aResult.ParseIntWithBounds(aString, 0);
+    return aResult.ParseNonNegativeIntValue(aString);
   }
   return false;
 }
 
 bool nsGenericHTMLElement::ParseReferrerAttribute(const nsAString& aString,
                                                   nsAttrValue& aResult) {
   static const nsAttrValue::EnumTable kReferrerTable[] = {
       {ReferrerPolicyToString(net::RP_No_Referrer),
--- a/testing/web-platform/tests/html/rendering/pixel-length-attributes.html
+++ b/testing/web-platform/tests/html/rendering/pixel-length-attributes.html
@@ -1,16 +1,21 @@
 <!doctype html>
 <meta charset=utf-8>
 <title>Test handling of attributes that map to pixel length properties</title>
 <link rel="help"
       href="https://html.spec.whatwg.org/multipage/rendering.html#maps-to-the-pixel-length-property">
 <script src=/resources/testharness.js></script>
 <script src=/resources/testharnessreport.js></script>
 <body>
+  <div id="container" style="display: none">
+    <img id="defaultImg">
+    <object id="defaultObject"></object>
+    <input type="image" id="defaultInput"></input>
+  </div>
 <script>
  /*
   * This test tests
   * https://html.spec.whatwg.org/multipage/rendering.html#maps-to-the-pixel-length-property
   * for various elements and various values.
   */
 
  /*
@@ -72,18 +77,44 @@ const tests = [
   [ createBody, "marginwidth", "marginLeft", document.body ],
   [ createBody, "marginwidth", "marginRight", document.body ],
   [ createBody, "leftmargin", "marginLeft", document.body ],
   [ createBody, "rightmargin", "marginRight", document.body ],
   [ createBody, "marginheight", "marginTop", document.body ],
   [ createBody, "marginheight", "marginBottom", document.body ],
   [ createBody, "topmargin", "marginTop", document.body ],
   [ createBody, "bottommargin", "marginBottom", document.body ],
+  [ newElem("img"), "border", "borderTopWidth", defaultImg ],
+  [ newElem("img"), "border", "borderRightWidth", defaultImg ],
+  [ newElem("img"), "border", "borderBottomWidth", defaultImg ],
+  [ newElem("img"), "border", "borderLeftWidth", defaultImg ],
+  [ newElem("object"), "border", "borderTopWidth", defaultObject ],
+  [ newElem("object"), "border", "borderRightWidth", defaultObject ],
+  [ newElem("object"), "border", "borderBottomWidth", defaultObject ],
+  [ newElem("object"), "border", "borderLeftWidth", defaultObject ],
+  [ newImageInput, "border", "borderTopWidth", defaultInput ],
+  [ newImageInput, "border", "borderRightWidth", defaultInput ],
+  [ newImageInput, "border", "borderBottomWidth", defaultInput ],
+  [ newImageInput, "border", "borderLeftWidth", defaultInput ],
 ];
 
+function newElem(name) {
+  return () => {
+    var elem = document.createElement(name);
+    document.getElementById("container").appendChild(elem);
+    return [ elem, elem, () => elem.remove() ];
+  }
+}
+
+function newImageInput() {
+  var elem = document.createElement("input");
+  elem.type = "image";
+  document.getElementById("container").appendChild(elem);
+  return [ elem, elem, () => elem.remove() ];
+}
 
 function createIframe() {
   let ifr = document.createElement("iframe");
   document.body.appendChild(ifr);
   return [ ifr, ifr.contentDocument.body, () => ifr.remove() ];
  }
 
 function createBody() {
--- a/testing/web-platform/tests/html/rendering/unmapped-attributes.html
+++ b/testing/web-platform/tests/html/rendering/unmapped-attributes.html
@@ -39,16 +39,39 @@ function newElem(name) {
   * 2) The name of the attribute to set
   * 3) The name of the CSS property to get.
   */
 const tests = [
   [ newElem("table"), "hspace", "marginLeft" ],
   [ newElem("table"), "hspace", "marginRight" ],
   [ newElem("table"), "vspace", "marginTop" ],
   [ newElem("table"), "vspace", "marginBottom" ],
+  [ newElem("embed"), "border", "borderTopWidth" ],
+  [ newElem("embed"), "border", "borderRightWidth" ],
+  [ newElem("embed"), "border", "borderBottomWidth" ],
+  [ newElem("embed"), "border", "borderLeftWidth" ],
+  [ newElem("iframe"), "border", "borderTopWidth" ],
+  [ newElem("iframe"), "border", "borderRightWidth" ],
+  [ newElem("iframe"), "border", "borderBottomWidth" ],
+  [ newElem("iframe"), "border", "borderLeftWidth" ],
+  [ newElem("marquee"), "border", "borderTopWidth" ],
+  [ newElem("marquee"), "border", "borderRightWidth" ],
+  [ newElem("marquee"), "border", "borderBottomWidth" ],
+  [ newElem("marquee"), "border", "borderLeftWidth" ],
+  // Non-image input
+  [ newElem("input"), "border", "borderTopWidth" ],
+  [ newElem("input"), "border", "borderRightWidth" ],
+  [ newElem("input"), "border", "borderBottomWidth" ],
+  [ newElem("input"), "border", "borderLeftWidth" ],
+  [ newElem("input"), "width", "width" ],
+  [ newElem("input"), "height", "height" ],
+  [ newElem("input"), "hspace", "marginLeft" ],
+  [ newElem("input"), "hspace", "marginRight" ],
+  [ newElem("input"), "vspace", "marginTop" ],
+  [ newElem("input"), "vspace", "marginBottom" ],
 ];
 
 function style(element) {
   return element.ownerDocument.defaultView.getComputedStyle(element);
 }
 
 for (let [ctor, attr, prop] of tests) {
   for (let parent of [container, frameContainer]) {