Title: [171941] trunk
Revision
171941
Author
mmaxfi...@apple.com
Date
2014-08-01 12:08:59 -0700 (Fri, 01 Aug 2014)

Log Message

URLs in srcset attributes are not made absolute upon copy and paste
https://bugs.webkit.org/show_bug.cgi?id=135448

Reviewed by Ryosuke Niwa.

Source/WebCore:
When pasting, canonicalize URLs in srcset the same way we do with src.

Test: editing/pasteboard/img-srcset-copy-paste-canonicalization.html

* dom/Element.cpp:
(WebCore::Element::completeURLsInAttributeValue): Initial implemention, moved from markup.cpp.
* dom/Element.h:
(WebCore::Element::attributeContainsURL): New function for completeURLs to call.
(WebCore::Element::completeURLsInAttributeValue): Only called if attributeContainsURL returns
true. Default implementation simply calls isURLAttribute().
* editing/markup.cpp:
(WebCore::completeURLs): Call attributeContainsURL() and completeURLsInAttributeValue() to
complete the URL, so nodes can perform their own behavior.
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::attributeContainsURL): Return true for srcset.
(WebCore::HTMLImageElement::completeUrlAttributeValue): Use our existing srcset parser to
parse the srcset attribute, then use its output to canonicalize URLs, and build it back up
into a string.
* html/HTMLImageElement.h:
(WebCore::HTMLImageElement::attributeContainsURL):
(WebCore::HTMLImageElement::completeUrlAttributeValue):
* html/parser/HTMLSrcsetParser.cpp: Make parseImageCandidatesFromSrcsetAttribute() public
and change its signature to return its result.
(WebCore::parseImageCandidatesFromSrcsetAttribute):
* html/parser/HTMLSrcsetParser.h: Ditto.

LayoutTests:
Copy and paste a srcset image with relative URLs, and make sure that the
pasted srcset attribute doesn't match what it was before. I can't actually
dump the new srcset because it will include a full path of the file on the
user's system, and would therefore be machine-specific.

* editing/pasteboard/img-srcset-copy-paste-canonicalization-expected.txt:
* editing/pasteboard/img-srcset-copy-paste-canonicalization.html: Paste and check.
* editing/pasteboard/resources/img-srcset-copy-paste-canonicalization-iframe.html:
This has to be an iframe because we don't perform any url canonicalization if we
are copying and pasting from a document into itself.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (171940 => 171941)


--- trunk/LayoutTests/ChangeLog	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/LayoutTests/ChangeLog	2014-08-01 19:08:59 UTC (rev 171941)
@@ -1,3 +1,21 @@
+2014-07-30  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        URLs in srcset attributes are not made absolute upon copy and paste
+        https://bugs.webkit.org/show_bug.cgi?id=135448
+
+        Reviewed by Ryosuke Niwa.
+
+        Copy and paste a srcset image with relative URLs, and make sure that the
+        pasted srcset attribute doesn't match what it was before. I can't actually
+        dump the new srcset because it will include a full path of the file on the
+        user's system, and would therefore be machine-specific.
+
+        * editing/pasteboard/img-srcset-copy-paste-canonicalization-expected.txt:
+        * editing/pasteboard/img-srcset-copy-paste-canonicalization.html: Paste and check.
+        * editing/pasteboard/resources/img-srcset-copy-paste-canonicalization-iframe.html:
+        This has to be an iframe because we don't perform any url canonicalization if we
+        are copying and pasting from a document into itself.
+
 2014-08-01  Michał Pakuła vel Rutka  <m.pak...@samsung.com>
 
         Unreviewed EFL gardening

Added: trunk/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization-expected.txt (0 => 171941)


--- trunk/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization-expected.txt	2014-08-01 19:08:59 UTC (rev 171941)
@@ -0,0 +1,18 @@
+The following test does a copy and a paste of an image with the srcset attribute. The test is successful if the value of the srcset attribute has been canonicalized. To run the test manually, copy the green square into the contentediable area above the iframe.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS canonicalize(iframeHref, sourceImageSegments[0]) is destinationImageSegments[0]
+PASS canonicalize(iframeHref, sourceImageSegments[2]) is destinationImageSegments[2]
+PASS sourceImageSegments.length is 4
+PASS destinationImageSegments.length is 4
+PASS sourceImageSegments[1] is "2x,"
+PASS destinationImageSegments[1] is "2x,"
+PASS sourceImageSegments[3] is "1x"
+PASS destinationImageSegments[3] is "1x"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+

Added: trunk/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization.html (0 => 171941)


--- trunk/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization.html	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization.html	2014-08-01 19:08:59 UTC (rev 171941)
@@ -0,0 +1,76 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<div id="destination" contenteditable="true"></div>
+<iframe id="iframe" src=""
+<script>
+    window.jsTestIsAsync = true;
+
+    description("The following test does a copy and a paste of an image with the srcset attribute. The test is successful if the value of the srcset attribute has been canonicalized. To run the test manually, copy the green square into the contentediable area above the iframe.");
+    
+    function canonicalize(href, relativeURL) {
+        var hrefSegments = href.split("/");
+
+        // Remove the filename from the href
+        hrefSegments.splice(hrefSegments.length - 1, 1);
+
+        var relativeURLSegments = relativeURL.split("/");
+        while (relativeURLSegments.length > 0 && relativeURLSegments[0] == "..") {
+            hrefSegments.splice(hrefSegments.length - 1, 1);
+            relativeURLSegments.splice(0, 1);
+        }
+        return hrefSegments.concat(relativeURLSegments).join("/");
+    }
+
+    var iframeHref;
+    var sourceImageSegments;
+    var destinationImageSegments;
+
+    function runTests(href) {
+        iframeHref = href;
+
+        var iframe = document.getElementById("iframe");
+        var iframeDocument = iframe.contentDocument;
+
+        var source = iframeDocument.getElementById("source");
+        var destination = document.getElementById("destination");
+
+        source.focus();
+        iframeDocument.execCommand("SelectAll");
+        iframeDocument.execCommand("Copy");
+        destination.focus();
+        document.execCommand("SelectAll");
+        document.execCommand("Paste");
+
+        var sourceImage = iframeDocument.getElementById("image");
+        var destinationImage = document.getElementById("image");
+        if (!destinationImage) {
+            testFailed("Image was not pasted");
+            finishJSTest();
+            return;
+        }
+
+        destinationImage._onerror_ = function() {
+            testFailed("Image should not fail to load");
+            finishJSTest();
+        }
+        image._onload_ = function() {
+            finishJSTest();
+        }
+
+        sourceImageSegments = sourceImage.getAttribute("srcset").split(" ");
+        destinationImageSegments = destinationImage.getAttribute("srcset").split(" ");
+        shouldBe("canonicalize(iframeHref, sourceImageSegments[0])", "destinationImageSegments[0]");
+        shouldBe("canonicalize(iframeHref, sourceImageSegments[2])", "destinationImageSegments[2]");
+        shouldBe("sourceImageSegments.length", "4");
+        shouldBe("destinationImageSegments.length", "4");
+        shouldBe("sourceImageSegments[1]", "\"2x,\"");
+        shouldBe("destinationImageSegments[1]", "\"2x,\"");
+        shouldBe("sourceImageSegments[3]", "\"1x\"");
+        shouldBe("destinationImageSegments[3]", "\"1x\"");
+    }
+</script>
+<script src=""
+</body>
+</html>

Added: trunk/LayoutTests/editing/pasteboard/resources/img-srcset-copy-paste-canonicalization-iframe.html (0 => 171941)


--- trunk/LayoutTests/editing/pasteboard/resources/img-srcset-copy-paste-canonicalization-iframe.html	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/resources/img-srcset-copy-paste-canonicalization-iframe.html	2014-08-01 19:08:59 UTC (rev 171941)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body _onload_="window.parent.runTests(window.location.href);">
+<div id="source" contenteditable="true">
+    <img id="image" src="" srcset="../../../fast/hidpi/resources/green-400-px-square.png 2x, ../../../fast/hidpi/resources/green-200-px-square.png 1x">
+</div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (171940 => 171941)


--- trunk/Source/WebCore/ChangeLog	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/Source/WebCore/ChangeLog	2014-08-01 19:08:59 UTC (rev 171941)
@@ -1,3 +1,36 @@
+2014-07-30  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        URLs in srcset attributes are not made absolute upon copy and paste
+        https://bugs.webkit.org/show_bug.cgi?id=135448
+
+        Reviewed by Ryosuke Niwa.
+
+        When pasting, canonicalize URLs in srcset the same way we do with src.
+
+        Test: editing/pasteboard/img-srcset-copy-paste-canonicalization.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::completeURLsInAttributeValue): Initial implemention, moved from markup.cpp.
+        * dom/Element.h:
+        (WebCore::Element::attributeContainsURL): New function for completeURLs to call.
+        (WebCore::Element::completeURLsInAttributeValue): Only called if attributeContainsURL returns
+        true. Default implementation simply calls isURLAttribute().
+        * editing/markup.cpp:
+        (WebCore::completeURLs): Call attributeContainsURL() and completeURLsInAttributeValue() to
+        complete the URL, so nodes can perform their own behavior.
+        * html/HTMLImageElement.cpp:
+        (WebCore::HTMLImageElement::attributeContainsURL): Return true for srcset.
+        (WebCore::HTMLImageElement::completeUrlAttributeValue): Use our existing srcset parser to
+        parse the srcset attribute, then use its output to canonicalize URLs, and build it back up
+        into a string.
+        * html/HTMLImageElement.h:
+        (WebCore::HTMLImageElement::attributeContainsURL):
+        (WebCore::HTMLImageElement::completeUrlAttributeValue):
+        * html/parser/HTMLSrcsetParser.cpp: Make parseImageCandidatesFromSrcsetAttribute() public
+        and change its signature to return its result.
+        (WebCore::parseImageCandidatesFromSrcsetAttribute):
+        * html/parser/HTMLSrcsetParser.h: Ditto.
+
 2014-07-31  Andreas Kling  <akl...@apple.com>
 
         Remove the JSC::OverridesVisitChildren flag.

Modified: trunk/Source/WebCore/dom/Element.cpp (171940 => 171941)


--- trunk/Source/WebCore/dom/Element.cpp	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/Source/WebCore/dom/Element.cpp	2014-08-01 19:08:59 UTC (rev 171941)
@@ -3022,4 +3022,9 @@
     return !equalIgnoringCase(fastGetAttribute(roleAttr), "img");
 }
 
+String Element::completeURLsInAttributeValue(const URL& base, const Attribute& attribute) const
+{
+    return URL(base, attribute.value()).string();
+}
+
 } // namespace WebCore

Modified: trunk/Source/WebCore/dom/Element.h (171940 => 171941)


--- trunk/Source/WebCore/dom/Element.h	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/Source/WebCore/dom/Element.h	2014-08-01 19:08:59 UTC (rev 171941)
@@ -393,6 +393,8 @@
     virtual void accessKeyAction(bool /*sendToAnyEvent*/) { }
 
     virtual bool isURLAttribute(const Attribute&) const { return false; }
+    virtual bool attributeContainsURL(const Attribute& attribute) const { return isURLAttribute(attribute); }
+    virtual String completeURLsInAttributeValue(const URL& base, const Attribute&) const;
     virtual bool isHTMLContentAttribute(const Attribute&) const { return false; }
 
     URL getURLAttribute(const QualifiedName&) const;

Modified: trunk/Source/WebCore/editing/markup.cpp (171940 => 171941)


--- trunk/Source/WebCore/editing/markup.cpp	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/Source/WebCore/editing/markup.cpp	2014-08-01 19:08:59 UTC (rev 171941)
@@ -104,8 +104,8 @@
         if (!element.hasAttributes())
             continue;
         for (const Attribute& attribute : element.attributesIterator()) {
-            if (element.isURLAttribute(attribute) && !attribute.value().isEmpty())
-                changes.append(AttributeChange(&element, attribute.name(), URL(parsedBaseURL, attribute.value()).string()));
+            if (element.attributeContainsURL(attribute) && !attribute.value().isEmpty())
+                changes.append(AttributeChange(&element, attribute.name(), element.completeURLsInAttributeValue(parsedBaseURL, attribute)));
         }
     }
 

Modified: trunk/Source/WebCore/html/HTMLImageElement.cpp (171940 => 171941)


--- trunk/Source/WebCore/html/HTMLImageElement.cpp	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/Source/WebCore/html/HTMLImageElement.cpp	2014-08-01 19:08:59 UTC (rev 171941)
@@ -39,6 +39,7 @@
 #include "Settings.h"
 #include "ShadowRoot.h"
 #include "SourceSizeList.h"
+#include <wtf/text/StringBuilder.h>
 
 #if ENABLE(SERVICE_CONTROLS)
 #include "ImageControlsRootElement.h"
@@ -353,6 +354,37 @@
         || HTMLElement::isURLAttribute(attribute);
 }
 
+bool HTMLImageElement::attributeContainsURL(const Attribute& attribute) const
+{
+    return attribute.name() == srcsetAttr
+        || HTMLElement::attributeContainsURL(attribute);
+}
+
+String HTMLImageElement::completeURLsInAttributeValue(const URL& base, const Attribute& attribute) const
+{
+    if (attribute.name() == srcsetAttr) {
+        Vector<ImageCandidate> imageCandidates = parseImageCandidatesFromSrcsetAttribute(StringView(attribute.value()));
+        StringBuilder result;
+        for (const auto& candidate : imageCandidates) {
+            if (&candidate != &imageCandidates[0])
+                result.appendLiteral(", ");
+            result.append(URL(base, candidate.string.toString()).string());
+            if (candidate.density != UninitializedDescriptor) {
+                result.append(' ');
+                result.appendNumber(candidate.density);
+                result.append('x');
+            }
+            if (candidate.resourceWidth != UninitializedDescriptor) {
+                result.append(' ');
+                result.appendNumber(candidate.resourceWidth);
+                result.append('x');
+            }
+        }
+        return result.toString();
+    }
+    return HTMLElement::completeURLsInAttributeValue(base, attribute);
+}
+
 bool HTMLImageElement::matchesLowercasedUsemap(const AtomicStringImpl& name) const
 {
     ASSERT(String(&const_cast<AtomicStringImpl&>(name)).lower().impl() == &name);

Modified: trunk/Source/WebCore/html/HTMLImageElement.h (171940 => 171941)


--- trunk/Source/WebCore/html/HTMLImageElement.h	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/Source/WebCore/html/HTMLImageElement.h	2014-08-01 19:08:59 UTC (rev 171941)
@@ -108,6 +108,8 @@
     virtual bool canStartSelection() const override;
 
     virtual bool isURLAttribute(const Attribute&) const override;
+    virtual bool attributeContainsURL(const Attribute&) const override;
+    virtual String completeURLsInAttributeValue(const URL& base, const Attribute&) const override;
 
     virtual bool draggable() const override;
 

Modified: trunk/Source/WebCore/html/parser/HTMLSrcsetParser.cpp (171940 => 171941)


--- trunk/Source/WebCore/html/parser/HTMLSrcsetParser.cpp	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/Source/WebCore/html/parser/HTMLSrcsetParser.cpp	2014-08-01 19:08:59 UTC (rev 171941)
@@ -162,8 +162,10 @@
 
 // http://picture.responsiveimages.org/#parse-srcset-attr
 template<typename CharType>
-static void parseImageCandidatesFromSrcsetAttribute(const CharType* attributeStart, unsigned length, Vector<ImageCandidate>& imageCandidates)
+static Vector<ImageCandidate> parseImageCandidatesFromSrcsetAttribute(const CharType* attributeStart, unsigned length)
 {
+    Vector<ImageCandidate> imageCandidates;
+
     const CharType* attributeEnd = attributeStart + length;
 
     for (const CharType* position = attributeStart; position < attributeEnd;) {
@@ -207,15 +209,16 @@
         imageCandidates.append(ImageCandidate(StringView(imageURLStart, imageURLLength), result, ImageCandidate::SrcsetOrigin));
         // 11. Return to the step labeled splitting loop.
     }
+    return imageCandidates;
 }
 
-static void parseImageCandidatesFromSrcsetAttribute(StringView attribute, Vector<ImageCandidate>& imageCandidates)
+Vector<ImageCandidate> parseImageCandidatesFromSrcsetAttribute(StringView attribute)
 {
     // FIXME: We should consider replacing the direct pointers in the parsing process with StringView and positions.
     if (attribute.is8Bit())
-        parseImageCandidatesFromSrcsetAttribute<LChar>(attribute.characters8(), attribute.length(), imageCandidates);
+        return parseImageCandidatesFromSrcsetAttribute<LChar>(attribute.characters8(), attribute.length());
     else
-        parseImageCandidatesFromSrcsetAttribute<UChar>(attribute.characters16(), attribute.length(), imageCandidates);
+        return parseImageCandidatesFromSrcsetAttribute<UChar>(attribute.characters16(), attribute.length());
 }
 
 static ImageCandidate pickBestImageCandidate(float deviceScaleFactor, Vector<ImageCandidate>& imageCandidates
@@ -275,10 +278,8 @@
         return ImageCandidate(StringView(srcAttribute), DescriptorParsingResult(), ImageCandidate::SrcOrigin);
     }
 
-    Vector<ImageCandidate> imageCandidates;
+    Vector<ImageCandidate> imageCandidates = parseImageCandidatesFromSrcsetAttribute(StringView(srcsetAttribute));
 
-    parseImageCandidatesFromSrcsetAttribute(StringView(srcsetAttribute), imageCandidates);
-
     if (!srcAttribute.isEmpty())
         imageCandidates.append(ImageCandidate(StringView(srcAttribute), DescriptorParsingResult(), ImageCandidate::SrcOrigin));
 

Modified: trunk/Source/WebCore/html/parser/HTMLSrcsetParser.h (171940 => 171941)


--- trunk/Source/WebCore/html/parser/HTMLSrcsetParser.h	2014-08-01 19:06:55 UTC (rev 171940)
+++ trunk/Source/WebCore/html/parser/HTMLSrcsetParser.h	2014-08-01 19:08:59 UTC (rev 171941)
@@ -104,6 +104,8 @@
 #endif
     );
 
+Vector<ImageCandidate> parseImageCandidatesFromSrcsetAttribute(StringView attribute);
+
 }
 
 #endif
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to