URLs in srcset attributes are not made absolute upon copy and paste
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Aug 2014 19:08:59 +0000 (19:08 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Aug 2014 19:08:59 +0000 (19:08 +0000)
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.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/resources/img-srcset-copy-paste-canonicalization-iframe.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/editing/markup.cpp
Source/WebCore/html/HTMLImageElement.cpp
Source/WebCore/html/HTMLImageElement.h
Source/WebCore/html/parser/HTMLSrcsetParser.cpp
Source/WebCore/html/parser/HTMLSrcsetParser.h

index 9b0bc9564a60d00872142ffbaeb99a5f0246bebf..1247ed89013e46f95ae5644b7e7fb6440f431729 100644 (file)
@@ -1,3 +1,21 @@
+2014-07-30  Myles C. Maxfield  <mmaxfield@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.pakula@samsung.com>
 
         Unreviewed EFL gardening
diff --git a/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization-expected.txt b/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization-expected.txt
new file mode 100644 (file)
index 0000000..9e395a4
--- /dev/null
@@ -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
+
+
diff --git a/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization.html b/LayoutTests/editing/pasteboard/img-srcset-copy-paste-canonicalization.html
new file mode 100644 (file)
index 0000000..efb6797
--- /dev/null
@@ -0,0 +1,76 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<div id="destination" contenteditable="true"></div>
+<iframe id="iframe" src="resources/img-srcset-copy-paste-canonicalization-iframe.html"></iframe>
+<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="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/editing/pasteboard/resources/img-srcset-copy-paste-canonicalization-iframe.html b/LayoutTests/editing/pasteboard/resources/img-srcset-copy-paste-canonicalization-iframe.html
new file mode 100644 (file)
index 0000000..0724cbc
--- /dev/null
@@ -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="../../../fast/hidpi/resources/srcset.png" srcset="../../../fast/hidpi/resources/green-400-px-square.png 2x, ../../../fast/hidpi/resources/green-200-px-square.png 1x">
+</div>
+</body>
+</html>
index ec0ba923d98609fc57ded72b031faaac96e201bd..c445d9bd4951550200fc88070a3e917c8fe90b75 100644 (file)
@@ -1,3 +1,36 @@
+2014-07-30  Myles C. Maxfield  <mmaxfield@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  <akling@apple.com>
 
         Remove the JSC::OverridesVisitChildren flag.
index 202400b7877eac4927dfb9899ce50d7079bc4964..aac5d65bcc74bb40321b149ac93bfe8163eee454 100644 (file)
@@ -3022,4 +3022,9 @@ bool Element::canContainRangeEndPoint() const
     return !equalIgnoringCase(fastGetAttribute(roleAttr), "img");
 }
 
+String Element::completeURLsInAttributeValue(const URL& base, const Attribute& attribute) const
+{
+    return URL(base, attribute.value()).string();
+}
+
 } // namespace WebCore
index ae9459488f35daeda276d71370a38f002fd08f1a..e5e97c14e47976d020dc1d19fcb116f088c85203 100644 (file)
@@ -393,6 +393,8 @@ public:
     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;
index 7d6a7f6bdbe0ae555ca945bd52511c2a278720fb..d7e42f936afcd0ad1ed21cba6dcf51611f210ff9 100644 (file)
@@ -104,8 +104,8 @@ static void completeURLs(DocumentFragment* fragment, const String& baseURL)
         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)));
         }
     }
 
index 60d04e428d5d7ae285dcf1379a8e8d605901a237..f700939680df5af5ba95fec005f1ea2620634b9a 100644 (file)
@@ -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 @@ bool HTMLImageElement::isURLAttribute(const Attribute& attribute) const
         || 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);
index 844c4702b3ce533bd5e0c39662aff4099420731f..7cfbf70ecb24d6d231b92c1f4077cec529621aba 100644 (file)
@@ -108,6 +108,8 @@ private:
     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;
 
index 12929ae5f0ac807cc7dd27e63127ff130bfe4c70..e26961fd0ed89c2da9e8883c50c997474e6c8428 100644 (file)
@@ -162,8 +162,10 @@ static bool parseDescriptors(Vector<StringView>& descriptors, DescriptorParsingR
 
 // 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 @@ static void parseImageCandidatesFromSrcsetAttribute(const CharType* attributeSta
         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,9 +278,7 @@ ImageCandidate bestFitSourceForImageAttributes(float deviceScaleFactor, const At
         return ImageCandidate(StringView(srcAttribute), DescriptorParsingResult(), ImageCandidate::SrcOrigin);
     }
 
-    Vector<ImageCandidate> imageCandidates;
-
-    parseImageCandidatesFromSrcsetAttribute(StringView(srcsetAttribute), imageCandidates);
+    Vector<ImageCandidate> imageCandidates = parseImageCandidatesFromSrcsetAttribute(StringView(srcsetAttribute));
 
     if (!srcAttribute.isEmpty())
         imageCandidates.append(ImageCandidate(StringView(srcAttribute), DescriptorParsingResult(), ImageCandidate::SrcOrigin));
index bc1224f8bf42a66062fd2d8fd0bced7381cd2fb6..7b6edafc89a3b2f8a12593a13087f8e87ee1b785 100644 (file)
@@ -104,6 +104,8 @@ ImageCandidate bestFitSourceForImageAttributes(float deviceScaleFactor, const At
 #endif
     );
 
+Vector<ImageCandidate> parseImageCandidatesFromSrcsetAttribute(StringView attribute);
+
 }
 
 #endif