HTMLAreaElement's coords attributes parsing does not comply with the HTML specification
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Aug 2016 18:09:01 +0000 (18:09 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Aug 2016 18:09:01 +0000 (18:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161216

Reviewed by Andreas Kling.

LayoutTests/imported/w3c:

Rebaseline W3C test now that all checks are passing. Chrome is also passing all the checks.

* web-platform-tests/html/semantics/embedded-content/the-area-element/area-coords-expected.txt:

Source/WebCore:

HTMLAreaElement's coords attributes parsing does not comply with the HTML specification:
- https://html.spec.whatwg.org/#attr-area-coords

This patch aligns our parsing of this attribute with the specification and Chrome.

No new tests, rebaselined existing test.

* html/HTMLAreaElement.cpp:
(WebCore::HTMLAreaElement::HTMLAreaElement):
(WebCore::HTMLAreaElement::parseAttribute):
(WebCore::HTMLAreaElement::getRegion):
* html/HTMLAreaElement.h:
* html/parser/HTMLParserIdioms.cpp:
(WebCore::isHTMLSpaceOrDelimiter):
(WebCore::isNumberStart):
(WebCore::parseHTMLListOfOfFloatingPointNumberValuesInternal):
(WebCore::parseHTMLListOfOfFloatingPointNumberValues):
(WebCore::parseHTMLNonNegativeInteger): Deleted.
* html/parser/HTMLParserIdioms.h:

LayoutTests:

Unskip web-platform-tests/html/semantics/embedded-content/the-area-element/area-coords.html
as it is now passing and not longer crashing in debug builds.

* TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@205030 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/html/semantics/embedded-content/the-area-element/area-coords-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLAreaElement.cpp
Source/WebCore/html/HTMLAreaElement.h
Source/WebCore/html/parser/HTMLParserIdioms.cpp
Source/WebCore/html/parser/HTMLParserIdioms.h

index a1329d7..c7f36f4 100644 (file)
@@ -1,5 +1,17 @@
 2016-08-26  Chris Dumez  <cdumez@apple.com>
 
 2016-08-26  Chris Dumez  <cdumez@apple.com>
 
+        HTMLAreaElement's coords attributes parsing does not comply with the HTML specification
+        https://bugs.webkit.org/show_bug.cgi?id=161216
+
+        Reviewed by Andreas Kling.
+
+        Unskip web-platform-tests/html/semantics/embedded-content/the-area-element/area-coords.html
+        as it is now passing and not longer crashing in debug builds.
+
+        * TestExpectations:
+
+2016-08-26  Chris Dumez  <cdumez@apple.com>
+
         Trying to access cross-origin Location properties should throw a SecurityError
         https://bugs.webkit.org/show_bug.cgi?id=161248
 
         Trying to access cross-origin Location properties should throw a SecurityError
         https://bugs.webkit.org/show_bug.cgi?id=161248
 
index 52e23ea..10fe6ec 100644 (file)
@@ -475,8 +475,6 @@ webkit.org/b/154310 [ Debug ] imported/w3c/web-platform-tests/html/dom/reflectio
 # WPT tests that fail after doing full test repository reimport and need further investigation
 imported/w3c/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm [ Pass Failure Timeout ]
 imported/w3c/web-platform-tests/dom/nodes/Document-createElement-namespace.html [ Pass Failure ]
 # WPT tests that fail after doing full test repository reimport and need further investigation
 imported/w3c/web-platform-tests/XMLHttpRequest/send-entity-body-document.htm [ Pass Failure Timeout ]
 imported/w3c/web-platform-tests/dom/nodes/Document-createElement-namespace.html [ Pass Failure ]
-webkit.org/b/155516 [ Debug ] imported/w3c/web-platform-tests/html/semantics/embedded-content/the-area-element/area-coords.html [ Skip ]
-webkit.org/b/155516 [ Release ] imported/w3c/web-platform-tests/html/semantics/embedded-content/the-area-element/area-coords.html [ Failure ]
 
 # W3C XMLHttpRequest tests
 imported/w3c/web-platform-tests/XMLHttpRequest/xmlhttprequest-timeout-aborted.html [ Slow ]
 
 # W3C XMLHttpRequest tests
 imported/w3c/web-platform-tests/XMLHttpRequest/xmlhttprequest-timeout-aborted.html [ Slow ]
index f3247b1..01c98cb 100644 (file)
@@ -1,3 +1,14 @@
+2016-08-26  Chris Dumez  <cdumez@apple.com>
+
+        HTMLAreaElement's coords attributes parsing does not comply with the HTML specification
+        https://bugs.webkit.org/show_bug.cgi?id=161216
+
+        Reviewed by Andreas Kling.
+
+        Rebaseline W3C test now that all checks are passing. Chrome is also passing all the checks.
+
+        * web-platform-tests/html/semantics/embedded-content/the-area-element/area-coords-expected.txt:
+
 2016-08-25  Johan K. Jensen  <johan_jensen@apple.com>
 
         Update the Resource Timing implementation
 2016-08-25  Johan K. Jensen  <johan_jensen@apple.com>
 
         Update the Resource Timing implementation
index ae5aded..6977e60 100644 (file)
@@ -7,9 +7,29 @@ PASS TAB: "2\t2\t10\t10" (rect)
 PASS FORM FEED: "2\f2\f10\f10" (rect) 
 PASS LINE FEED: "2\n2\n10\n10" (rect) 
 PASS CARRIGAGE RETURN: "2\r2\r10\r10" (rect) 
 PASS FORM FEED: "2\f2\f10\f10" (rect) 
 PASS LINE FEED: "2\n2\n10\n10" (rect) 
 PASS CARRIGAGE RETURN: "2\r2\r10\r10" (rect) 
-FAIL LINE TABULATION: "2\v2\v10\v10" (rect) assert_equals: elementFromPoint(3, 3) expected Element node <img src="/images/threecolors.png" usemap="#x" id="img" w... but got Element node <area id="area" shape="rect" coords="2\v2\v10\v10"></area>
-FAIL LINE NEXT: "2\852\8510\8510" (rect) assert_equals: elementFromPoint(3, 3) expected Element node <img src="/images/threecolors.png" usemap="#x" id="img" w... but got Element node <area id="area" shape="rect" coords="2\852\8510\8510"></area>
-FAIL EN QUAD: "2 2 10 10" (rect) assert_equals: elementFromPoint(3, 3) expected Element node <img src="/images/threecolors.png" usemap="#x" id="img" w... but got Element node <area id="area" shape="rect" coords="2 2 10 10"></area>
-FAIL abc between numbers: "2a2b20c20,2,10,10" (rect) assert_equals: elementFromPoint(11, 9) expected Element node <img src="/images/threecolors.png" usemap="#x" id="img" w... but got Element node <area id="area" shape="rect" coords="2a2b20c20,2,10,10"><...
-FAIL COLON between numbers: "2:2:20:20,2,10,10" (rect) assert_equals: elementFromPoint(11, 9) expected Element node <img src="/images/threecolors.png" usemap="#x" id="img" w... but got Element node <area id="area" shape="rect" coords="2:2:20:20,2,10,10"><...
-FAIL U+0000 between numbers: "2\02\020\020,2,10,10" (rect) assert_equals: elementFromPoint(11, 9) expected Element node <img src="/images/threecolors.png" usemap="#x" id="img" w... but got Element node <area id="area" shape="rect" coords="2
\ No newline at end of file
+PASS LINE TABULATION: "2\v2\v10\v10" (rect) 
+PASS LINE NEXT: "2\852\8510\8510" (rect) 
+PASS EN QUAD: "2 2 10 10" (rect) 
+PASS abc between numbers: "2a2b20c20,2,10,10" (rect) 
+PASS COLON between numbers: "2:2:20:20,2,10,10" (rect) 
+PASS U+0000 between numbers: "2\02\020\020,2,10,10" (rect) 
+PASS leading COMMA: ",2,2,10,10" (rect) 
+PASS leading SPACE: " 2,2,10,10" (rect) 
+PASS leading SEMICOLON: ";2,2,10,10" (rect) 
+PASS trailing COMMA: "2,2,10," (rect) 
+PASS trailing SPACE: "2,2,10 " (rect) 
+PASS trailing SEMICOLON: "2,2,10;" (rect) 
+PASS PERCENT: "2%,2%,10%,10%" (rect) 
+PASS CSS units: "2in,2in,10cm,10cm" (rect) 
+PASS float: "1.4,1.4,10,10" (rect) 
+PASS number starting with PERIOD: ".4,.4,10,10" (rect) 
+PASS sci-not: "2,2,1e1,1e1" (rect) 
+PASS leading/trailing garbage: "='2,2,10,10' " (rect) 
+PASS non-ascii garbage: "“2,2,10,10\"" (rect) 
+PASS URL garbage with number: "2,2,10ls/spain/holidays/regions/10/Canary+Islands/Canary+Islands.html" (rect) 
+PASS consecutive COMMAs: "2,,10,10" (rect) 
+PASS consecutive SPACEs: "2  10,10" (rect) 
+PASS consecutive SEMICOLONs: "2;;10,10" (rect) 
+PASS several consecutive separators: ",,2;,;2,;,10 \t\r\n10;;" (rect) 
+PASS one too many numbers, trailing COMMA: "100,100,120,100,100,120,300," (poly) 
+
index 2978ed9..4335cb3 100644 (file)
@@ -1,3 +1,30 @@
+2016-08-26  Chris Dumez  <cdumez@apple.com>
+
+        HTMLAreaElement's coords attributes parsing does not comply with the HTML specification
+        https://bugs.webkit.org/show_bug.cgi?id=161216
+
+        Reviewed by Andreas Kling.
+
+        HTMLAreaElement's coords attributes parsing does not comply with the HTML specification:
+        - https://html.spec.whatwg.org/#attr-area-coords
+
+        This patch aligns our parsing of this attribute with the specification and Chrome.
+
+        No new tests, rebaselined existing test.
+
+        * html/HTMLAreaElement.cpp:
+        (WebCore::HTMLAreaElement::HTMLAreaElement):
+        (WebCore::HTMLAreaElement::parseAttribute):
+        (WebCore::HTMLAreaElement::getRegion):
+        * html/HTMLAreaElement.h:
+        * html/parser/HTMLParserIdioms.cpp:
+        (WebCore::isHTMLSpaceOrDelimiter):
+        (WebCore::isNumberStart):
+        (WebCore::parseHTMLListOfOfFloatingPointNumberValuesInternal):
+        (WebCore::parseHTMLListOfOfFloatingPointNumberValues):
+        (WebCore::parseHTMLNonNegativeInteger): Deleted.
+        * html/parser/HTMLParserIdioms.h:
+
 2016-08-26  Csaba Osztrogonác  <ossy@webkit.org>
 
         Fix the !ENABLE(WEB_SOCKETS) build
 2016-08-26  Csaba Osztrogonác  <ossy@webkit.org>
 
         Fix the !ENABLE(WEB_SOCKETS) build
index 4b3a529..8501f9c 100644 (file)
@@ -26,6 +26,7 @@
 #include "Frame.h"
 #include "HTMLImageElement.h"
 #include "HTMLMapElement.h"
 #include "Frame.h"
 #include "HTMLImageElement.h"
 #include "HTMLMapElement.h"
+#include "HTMLParserIdioms.h"
 #include "HitTestResult.h"
 #include "Path.h"
 #include "RenderImage.h"
 #include "HitTestResult.h"
 #include "Path.h"
 #include "RenderImage.h"
@@ -37,7 +38,6 @@ using namespace HTMLNames;
 
 inline HTMLAreaElement::HTMLAreaElement(const QualifiedName& tagName, Document& document)
     : HTMLAnchorElement(tagName, document)
 
 inline HTMLAreaElement::HTMLAreaElement(const QualifiedName& tagName, Document& document)
     : HTMLAnchorElement(tagName, document)
-    , m_coordsLen(0)
     , m_lastSize(-1, -1)
     , m_shape(Unknown)
 {
     , m_lastSize(-1, -1)
     , m_shape(Unknown)
 {
@@ -62,7 +62,7 @@ void HTMLAreaElement::parseAttribute(const QualifiedName& name, const AtomicStri
             m_shape = Rect;
         invalidateCachedRegion();
     } else if (name == coordsAttr) {
             m_shape = Rect;
         invalidateCachedRegion();
     } else if (name == coordsAttr) {
-        m_coords = newCoordsArray(value.string(), m_coordsLen);
+        m_coords = parseHTMLListOfOfFloatingPointNumberValues(value.string());
         invalidateCachedRegion();
     } else if (name == altAttr || name == accesskeyAttr) {
         // Do nothing.
         invalidateCachedRegion();
     } else if (name == altAttr || name == accesskeyAttr) {
         // Do nothing.
@@ -129,7 +129,7 @@ LayoutRect HTMLAreaElement::computeRect(RenderObject* obj) const
 
 Path HTMLAreaElement::getRegion(const LayoutSize& size) const
 {
 
 Path HTMLAreaElement::getRegion(const LayoutSize& size) const
 {
-    if (!m_coords && m_shape != Default)
+    if (m_coords.isEmpty() && m_shape != Default)
         return Path();
 
     LayoutUnit width = size.width();
         return Path();
 
     LayoutUnit width = size.width();
@@ -138,38 +138,37 @@ Path HTMLAreaElement::getRegion(const LayoutSize& size) const
     // If element omits the shape attribute, select shape based on number of coordinates.
     Shape shape = m_shape;
     if (shape == Unknown) {
     // If element omits the shape attribute, select shape based on number of coordinates.
     Shape shape = m_shape;
     if (shape == Unknown) {
-        if (m_coordsLen == 3)
+        if (m_coords.size() == 3)
             shape = Circle;
             shape = Circle;
-        else if (m_coordsLen == 4)
+        else if (m_coords.size() == 4)
             shape = Rect;
             shape = Rect;
-        else if (m_coordsLen >= 6)
+        else if (m_coords.size() >= 6)
             shape = Poly;
     }
 
     Path path;
     switch (shape) {
         case Poly:
             shape = Poly;
     }
 
     Path path;
     switch (shape) {
         case Poly:
-            if (m_coordsLen >= 6) {
-                int numPoints = m_coordsLen / 2;
-                path.moveTo(FloatPoint(minimumValueForLength(m_coords[0], width), minimumValueForLength(m_coords[1], height)));
+            if (m_coords.size() >= 6) {
+                int numPoints = m_coords.size() / 2;
+                path.moveTo(FloatPoint(m_coords[0], m_coords[1]));
                 for (int i = 1; i < numPoints; ++i)
                 for (int i = 1; i < numPoints; ++i)
-                    path.addLineTo(FloatPoint(minimumValueForLength(m_coords[i * 2], width), minimumValueForLength(m_coords[i * 2 + 1], height)));
+                    path.addLineTo(FloatPoint(m_coords[i * 2], m_coords[i * 2 + 1]));
                 path.closeSubpath();
             }
             break;
         case Circle:
                 path.closeSubpath();
             }
             break;
         case Circle:
-            if (m_coordsLen >= 3) {
-                Length radius = m_coords[2];
-                int r = std::min(minimumValueForLength(radius, width), minimumValueForLength(radius, height));
-                path.addEllipse(FloatRect(minimumValueForLength(m_coords[0], width) - r, minimumValueForLength(m_coords[1], height) - r, 2 * r, 2 * r));
+            if (m_coords.size() >= 3) {
+                double r = m_coords[2];
+                path.addEllipse(FloatRect(m_coords[0] - r, m_coords[1] - r, 2 * r, 2 * r));
             }
             break;
         case Rect:
             }
             break;
         case Rect:
-            if (m_coordsLen >= 4) {
-                int x0 = minimumValueForLength(m_coords[0], width);
-                int y0 = minimumValueForLength(m_coords[1], height);
-                int x1 = minimumValueForLength(m_coords[2], width);
-                int y1 = minimumValueForLength(m_coords[3], height);
+            if (m_coords.size() >= 4) {
+                double x0 = m_coords[0];
+                double y0 = m_coords[1];
+                double x1 = m_coords[2];
+                double y1 = m_coords[3];
                 path.addRect(FloatRect(x0, y0, x1 - x0, y1 - y0));
             }
             break;
                 path.addRect(FloatRect(x0, y0, x1 - x0, y1 - y0));
             }
             break;
index bf813ba..4efe185 100644 (file)
@@ -65,8 +65,7 @@ private:
     void invalidateCachedRegion();
 
     std::unique_ptr<Path> m_region;
     void invalidateCachedRegion();
 
     std::unique_ptr<Path> m_region;
-    std::unique_ptr<Length[]> m_coords;
-    int m_coordsLen;
+    Vector<double> m_coords;
     LayoutSize m_lastSize;
     Shape m_shape;
 };
     LayoutSize m_lastSize;
     Shape m_shape;
 };
index a015ce6..ad332ee 100644 (file)
@@ -31,6 +31,7 @@
 #include <limits>
 #include <wtf/MathExtras.h>
 #include <wtf/NeverDestroyed.h>
 #include <limits>
 #include <wtf/MathExtras.h>
 #include <wtf/NeverDestroyed.h>
+#include <wtf/dtoa.h>
 #include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 #include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
@@ -215,6 +216,64 @@ Optional<int> parseHTMLNonNegativeInteger(const String& input)
     return signedValue;
 }
 
     return signedValue;
 }
 
+template <typename CharacterType>
+static inline bool isHTMLSpaceOrDelimiter(CharacterType character)
+{
+    return isHTMLSpace(character) || character == ',' || character == ';';
+}
+
+template <typename CharacterType>
+static inline bool isNumberStart(CharacterType character)
+{
+    return isASCIIDigit(character) || character == '.' || character == '-';
+}
+
+// https://html.spec.whatwg.org/multipage/infrastructure.html#rules-for-parsing-floating-point-number-values
+template <typename CharacterType>
+static Vector<double> parseHTMLListOfOfFloatingPointNumberValuesInternal(const CharacterType* position, const CharacterType* end)
+{
+    Vector<double> numbers;
+
+    // This skips past any leading delimiters.
+    while (position < end && isHTMLSpaceOrDelimiter(*position))
+        ++position;
+
+    while (position < end) {
+        // This skips past leading garbage.
+        while (position < end && !(isHTMLSpaceOrDelimiter(*position) || isNumberStart(*position)))
+            ++position;
+
+        const CharacterType* numberStart = position;
+        while (position < end && !isHTMLSpaceOrDelimiter(*position))
+            ++position;
+
+        size_t parsedLength = 0;
+        double number = parseDouble(numberStart, position - numberStart, parsedLength);
+        numbers.append(parsedLength > 0 && std::isfinite(number) ? number : 0);
+
+        // This skips past the delimiter.
+        while (position < end && isHTMLSpaceOrDelimiter(*position))
+            ++position;
+    }
+
+    return numbers;
+}
+
+Vector<double> parseHTMLListOfOfFloatingPointNumberValues(const String& input)
+{
+    unsigned length = input.length();
+    if (!length)
+        return { };
+
+    if (LIKELY(input.is8Bit())) {
+        auto* start = input.characters8();
+        return parseHTMLListOfOfFloatingPointNumberValuesInternal(start, start + length);
+    }
+
+    auto* start = input.characters16();
+    return parseHTMLListOfOfFloatingPointNumberValuesInternal(start, start + length);
+}
+
 static bool threadSafeEqual(const StringImpl& a, const StringImpl& b)
 {
     if (&a == &b)
 static bool threadSafeEqual(const StringImpl& a, const StringImpl& b)
 {
     if (&a == &b)
index 1d583cc..01aee40 100644 (file)
@@ -28,6 +28,7 @@
 #include <unicode/uchar.h>
 #include <wtf/Forward.h>
 #include <wtf/Optional.h>
 #include <unicode/uchar.h>
 #include <wtf/Forward.h>
 #include <wtf/Optional.h>
+#include <wtf/Vector.h>
 
 namespace WebCore {
 
 
 namespace WebCore {
 
@@ -66,6 +67,9 @@ WEBCORE_EXPORT Optional<int> parseHTMLInteger(const String&);
 // http://www.whatwg.org/specs/web-apps/current-work/#rules-for-parsing-non-negative-integers
 WEBCORE_EXPORT Optional<int> parseHTMLNonNegativeInteger(const String&);
 
 // http://www.whatwg.org/specs/web-apps/current-work/#rules-for-parsing-non-negative-integers
 WEBCORE_EXPORT Optional<int> parseHTMLNonNegativeInteger(const String&);
 
+// https://html.spec.whatwg.org/multipage/infrastructure.html#rules-for-parsing-floating-point-number-values
+Vector<double> parseHTMLListOfOfFloatingPointNumberValues(const String&);
+
 // https://html.spec.whatwg.org/multipage/infrastructure.html#cors-settings-attribute
 String parseCORSSettingsAttribute(const AtomicString&);
 
 // https://html.spec.whatwg.org/multipage/infrastructure.html#cors-settings-attribute
 String parseCORSSettingsAttribute(const AtomicString&);