Title: [164936] trunk
Revision
164936
Author
[email protected]
Date
2014-03-01 19:58:59 -0800 (Sat, 01 Mar 2014)

Log Message

Unreviewed, rolling out r164929 and r164934.
http://trac.webkit.org/changeset/164929
http://trac.webkit.org/changeset/164934
https://bugs.webkit.org/show_bug.cgi?id=129570

Caused assertions on two srcset tests (Requested by ap on
#webkit).

Source/WebCore:

* html/parser/HTMLParserIdioms.cpp:
(WebCore::isHTMLSpaceOrComma):
(WebCore::parseImagesWithScaleFromSrcsetAttribute):
(WebCore::bestFitSourceForImageAttributes):

LayoutTests:

* fast/hidpi/image-srcset-invalid-descriptor-expected.txt: Removed.
* fast/hidpi/image-srcset-invalid-descriptor.html: Removed.
* fast/hidpi/image-srcset-invalid-inputs-correct-src.html:
* fast/hidpi/image-srcset-src-selection-1x-both-expected.txt: Removed.
* fast/hidpi/image-srcset-src-selection-1x-both.html: Removed.
* fast/hidpi/resources/srcset-helper.js:
(runTest):

Modified Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (164935 => 164936)


--- trunk/LayoutTests/ChangeLog	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/LayoutTests/ChangeLog	2014-03-02 03:58:59 UTC (rev 164936)
@@ -1,3 +1,21 @@
+2014-03-01  Commit Queue  <[email protected]>
+
+        Unreviewed, rolling out r164929 and r164934.
+        http://trac.webkit.org/changeset/164929
+        http://trac.webkit.org/changeset/164934
+        https://bugs.webkit.org/show_bug.cgi?id=129570
+
+        Caused assertions on two srcset tests (Requested by ap on
+        #webkit).
+
+        * fast/hidpi/image-srcset-invalid-descriptor-expected.txt: Removed.
+        * fast/hidpi/image-srcset-invalid-descriptor.html: Removed.
+        * fast/hidpi/image-srcset-invalid-inputs-correct-src.html:
+        * fast/hidpi/image-srcset-src-selection-1x-both-expected.txt: Removed.
+        * fast/hidpi/image-srcset-src-selection-1x-both.html: Removed.
+        * fast/hidpi/resources/srcset-helper.js:
+        (runTest):
+
 2014-03-01  Filip Pizlo  <[email protected]>
 
         This shouldn't be a layout test since it runs only under jsc. Moving it to JSC

Deleted: trunk/LayoutTests/fast/hidpi/image-srcset-invalid-descriptor-expected.txt (164935 => 164936)


--- trunk/LayoutTests/fast/hidpi/image-srcset-invalid-descriptor-expected.txt	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/LayoutTests/fast/hidpi/image-srcset-invalid-descriptor-expected.txt	2014-03-02 03:58:59 UTC (rev 164936)
@@ -1,8 +0,0 @@
-PASS successfullyParsed is true
-
-TEST COMPLETE
-PASS document.getElementById("foo").clientWidth==100 is true
-PASS document.getElementById("foo2").clientWidth==200 is true
-This test passes if the image below is a 100px blue square when the deviceScaleFactor is 1.
-It tests that even though the 1x resource contains many invalid descriptors, only the invalid descriptors are ignored (according to the spec).
-  

Deleted: trunk/LayoutTests/fast/hidpi/image-srcset-invalid-descriptor.html (164935 => 164936)


--- trunk/LayoutTests/fast/hidpi/image-srcset-invalid-descriptor.html	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/LayoutTests/fast/hidpi/image-srcset-invalid-descriptor.html	2014-03-02 03:58:59 UTC (rev 164936)
@@ -1,23 +0,0 @@
-<html>
-<head>
-<script>
-    window.targetScaleFactor = 1;
-</script>
-<script src=""
-<script src=""
-<script>
-    addEventListener("load", function() {
-        shouldBeTrue('document.getElementById("foo").clientWidth==100');
-        shouldBeTrue('document.getElementById("foo2").clientWidth==200');
-    }, false);
-</script>
-</head>
-
-<body id="body">
-    <div>This test passes if the image below is a 100px blue square when the deviceScaleFactor is 1.</div>
-    <div> It tests that even though the 1x resource contains many invalid descriptors,
-        only the invalid descriptors are ignored (according to the spec).</div>
-    <img id="foo" src="" srcset="resources/blue-100-px-square.png 43q 1x dfs 3dd, resources/green-400-px-square.png 2x">
-    <img id="foo2" src="" srcset="resources/blue-100-px-square.png 300q, resources/green-400-px-square.png 2x">
-</body>
-</html>

Modified: trunk/LayoutTests/fast/hidpi/image-srcset-invalid-inputs-correct-src.html (164935 => 164936)


--- trunk/LayoutTests/fast/hidpi/image-srcset-invalid-inputs-correct-src.html	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/LayoutTests/fast/hidpi/image-srcset-invalid-inputs-correct-src.html	2014-03-02 03:58:59 UTC (rev 164936)
@@ -16,6 +16,6 @@
 <body id="body">
     <div>This test passes if this img tag below is a green square regardless of the scale factor. It ensures that invalid inputs
     from srcset are ignored, and src is selected (since it's the only candidate left)</div>
-    <img id="foo" src="" srcset="1x 5w,,  ,      , 2x  600h,,">
+    <img id="foo" src="" srcset="1x,,  ,      , 2x ,,">
 </body>
 </html>

Deleted: trunk/LayoutTests/fast/hidpi/image-srcset-src-selection-1x-both-expected.txt (164935 => 164936)


--- trunk/LayoutTests/fast/hidpi/image-srcset-src-selection-1x-both-expected.txt	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/LayoutTests/fast/hidpi/image-srcset-src-selection-1x-both-expected.txt	2014-03-02 03:58:59 UTC (rev 164936)
@@ -1,6 +0,0 @@
-PASS successfullyParsed is true
-
-TEST COMPLETE
-PASS document.getElementById("foo").clientWidth==400 is true
-This test passes if the div below is a blue 100px square when the deviceScaleFactor is 1. It simply ensures that when both src and srcset are specified for the same DPR, srcset wins.
-

Deleted: trunk/LayoutTests/fast/hidpi/image-srcset-src-selection-1x-both.html (164935 => 164936)


--- trunk/LayoutTests/fast/hidpi/image-srcset-src-selection-1x-both.html	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/LayoutTests/fast/hidpi/image-srcset-src-selection-1x-both.html	2014-03-02 03:58:59 UTC (rev 164936)
@@ -1,24 +0,0 @@
-<html>
-<head>
-<script>
-    window.targetScaleFactor = 2;
-</script>
-<script src=""
-<script src=""
-<script>
-    if (window.testRunner) {
-        testRunner.dumpAsText();
-    }
-
-    addEventListener("load", function() {
-        shouldBeTrue('document.getElementById("foo").clientWidth==400');
-    }, false);
-</script>
-</head>
-
-<body id="body">
-    <div>This test passes if the div below is a blue 100px square when the deviceScaleFactor is 1. 
-        It simply ensures that when both src and srcset are specified for the same DPR, srcset wins.</div>
-    <img id="foo" src="" srcset="resources/green-400-px-square.png 1x">
-</body>
-</html>

Modified: trunk/LayoutTests/fast/hidpi/resources/srcset-helper.js (164935 => 164936)


--- trunk/LayoutTests/fast/hidpi/resources/srcset-helper.js	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/LayoutTests/fast/hidpi/resources/srcset-helper.js	2014-03-02 03:58:59 UTC (rev 164936)
@@ -5,9 +5,6 @@
     if (!window.targetScaleFactor)
         window.targetScaleFactor = 2;
 
-    if(!sessionStorage.pageReloaded && window.targetScaleFactor == window.deviceScaleFactor)
-        return;
-
     if (!sessionStorage.scaleFactorIsSet) {
         testRunner.waitUntilDone();
         testRunner.setBackingScaleFactor(targetScaleFactor, scaleFactorIsSet);

Modified: trunk/Source/WebCore/ChangeLog (164935 => 164936)


--- trunk/Source/WebCore/ChangeLog	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/Source/WebCore/ChangeLog	2014-03-02 03:58:59 UTC (rev 164936)
@@ -1,3 +1,18 @@
+2014-03-01  Commit Queue  <[email protected]>
+
+        Unreviewed, rolling out r164929 and r164934.
+        http://trac.webkit.org/changeset/164929
+        http://trac.webkit.org/changeset/164934
+        https://bugs.webkit.org/show_bug.cgi?id=129570
+
+        Caused assertions on two srcset tests (Requested by ap on
+        #webkit).
+
+        * html/parser/HTMLParserIdioms.cpp:
+        (WebCore::isHTMLSpaceOrComma):
+        (WebCore::parseImagesWithScaleFromSrcsetAttribute):
+        (WebCore::bestFitSourceForImageAttributes):
+
 2014-03-01  Dan Bernstein  <[email protected]>
 
         Build fix.

Modified: trunk/Source/WebCore/html/parser/HTMLParserIdioms.cpp (164935 => 164936)


--- trunk/Source/WebCore/html/parser/HTMLParserIdioms.cpp	2014-03-02 03:45:06 UTC (rev 164935)
+++ trunk/Source/WebCore/html/parser/HTMLParserIdioms.cpp	2014-03-02 03:58:59 UTC (rev 164936)
@@ -297,38 +297,9 @@
     return first.scaleFactor() < second.scaleFactor();
 }
 
-static bool parseDescriptors(const String& attribute, size_t start, size_t end, float& imageScaleFactor)
+static inline bool isHTMLSpaceOrComma(UChar character)
 {
-    size_t descriptorStart;
-    size_t descriptorEnd;
-    size_t position = start;
-    bool isFoundScaleFactor = false;
-    bool isEmptyDescriptor = !(end > start);
-    bool isValid = false;
-
-    while (position < end) {
-        while (isHTMLSpace(attribute[position]) && position < end)
-            ++position;
-        descriptorStart = position;
-        while (isNotHTMLSpace(attribute[position]) && position < end)
-            ++position;
-        descriptorEnd = position;
-
-        ASSERT(descriptorEnd > descriptorStart);
-        --descriptorEnd;
-        // This part differs from the spec as the current implementation only supports pixel density descriptors.
-        if (attribute[descriptorEnd] != 'x')
-            continue;
-
-        if (isFoundScaleFactor)
-            return false;
-
-        imageScaleFactor = charactersToFloat(attribute.deprecatedCharacters() + descriptorStart, descriptorEnd - descriptorStart, &isValid);
-        isFoundScaleFactor = true;
-
-    }
-
-    return isEmptyDescriptor || isValid;
+    return isHTMLSpace(character) || character == ',';
 }
 
 // See the specifications for more details about the algorithm to follow.
@@ -364,15 +335,42 @@
         } else {
             // 7. Collect a sequence of characters that are not "," (U+002C) characters, and let that be descriptors.
             size_t imageScaleStart = srcsetAttribute.find(isNotHTMLSpace, imageURLEnd + 1);
-            imageScaleStart = (imageScaleStart == notFound) ? srcsetAttributeLength : imageScaleStart;
-            size_t imageScaleEnd = srcsetAttribute.find(',' , imageScaleStart + 1);
-            imageScaleEnd = (imageScaleEnd == notFound) ? srcsetAttributeLength : imageScaleEnd;
+            if (imageScaleStart == notFound)
+                separator = srcsetAttributeLength;
+            else if (srcsetAttribute[imageScaleStart] == ',')
+                separator = imageScaleStart;
+            else {
+                // This part differs from the spec as the current implementation only supports pixel density descriptors for now.
+                size_t imageScaleEnd = srcsetAttribute.find(isHTMLSpaceOrComma, imageScaleStart + 1);
+                imageScaleEnd = (imageScaleEnd == notFound) ? srcsetAttributeLength : imageScaleEnd;
+                size_t commaPosition = imageScaleEnd;
+                // Make sure there are no other descriptors.
+                while ((commaPosition < srcsetAttributeLength - 1) && isHTMLSpace(srcsetAttribute[commaPosition]))
+                    ++commaPosition;
+                // If the first not html space character after the scale modifier is not a comma,
+                // the current candidate is an invalid input.
+                if ((commaPosition < srcsetAttributeLength - 1) && srcsetAttribute[commaPosition] != ',') {
+                    // Find the nearest comma and skip the input.
+                    commaPosition = srcsetAttribute.find(',', commaPosition + 1);
+                    if (commaPosition == notFound)
+                        break;
+                    imageCandidateStart = commaPosition + 1;
+                    continue;
+                }
+                separator = commaPosition;
+                if (srcsetAttribute[imageScaleEnd - 1] != 'x') {
+                    imageCandidateStart = separator + 1;
+                    continue;
+                }
+                bool validScaleFactor = false;
+                size_t scaleFactorLengthWithoutUnit = imageScaleEnd - imageScaleStart - 1;
+                imageScaleFactor = charactersToFloat(srcsetAttribute.deprecatedCharacters() + imageScaleStart, scaleFactorLengthWithoutUnit, &validScaleFactor);
 
-            if (!parseDescriptors(srcsetAttribute, imageScaleStart, imageScaleEnd, imageScaleFactor)) {
-                imageCandidateStart = imageScaleEnd + 1;
-                continue;
+                if (!validScaleFactor) {
+                    imageCandidateStart = separator + 1;
+                    continue;
+                }
             }
-            separator = imageScaleEnd;
         }
         ImageWithScale image(imageURLStart, imageURLEnd - imageURLStart, imageScaleFactor);
         imageCandidates.append(image);
@@ -401,16 +399,8 @@
         if (imageCandidates[i].scaleFactor() >= deviceScaleFactor)
             return imageCandidates[i];
     }
-
-    // 16. If an entry b in candidates has the same associated ... pixel density as an earlier entry a in candidates,
-    // then remove entry b
-    size_t winner = imageCandidates.size() - 1;
-    size_t previousCandidate = winner;
-    float winningScaleFactor = imageCandidates.last().scaleFactor();
-    while ((previousCandidate > 0) && (imageCandidates[--previousCandidate].scaleFactor() == winningScaleFactor))
-        winner = previousCandidate;
-
-    return imageCandidates[winner];
+    const ImageWithScale& lastCandidate = imageCandidates.last();
+    return lastCandidate;
 }
 
 }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to