DevTools: correctly parse srcset attribute before linkifying urls

third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeElement.js

Index: third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeElement.js
diff --git a/third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeElement.js b/third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeElement.js
index 51268fdc6ff9eda688fd0e6c54cf2511a4ca9e1f..74d719f3923ee8ee8fa2586efc9f37ac246c70d0 100644
--- a/third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeElement.js
+++ b/third_party/WebKit/Source/devtools/front_end/elements/ElementsTreeElement.js
@@ -1281,37 +1281,59 @@ Elements.ElementsTreeElement = class extends UI.TreeElement {
       return link;
     }
 
-    if (node && (name === 'src' || name === 'href')) {
+    var nodeName = node ? node.nodeName().toLowerCase() : '';
+    if (node && (name === 'src' || name === 'href'))

[dgozman, 2017/05/08 23:37:40]

Use nodeName here.

[luoe, 2017/05/09 02:53:18]

Done.

       attrValueElement.appendChild(linkifyValue.call(this, value));
-    } else if (
-        node && (node.nodeName().toLowerCase() === 'img' || node.nodeName().toLowerCase() === 'source') &&
-        name === 'srcset') {
-      var sources = value.split(',');
-      for (var i = 0; i < sources.length; ++i) {
-        if (i > 0)
-          attrValueElement.createTextChild(', ');
-        var source = sources[i].trim();
-        var indexOfSpace = source.indexOf(' ');
-        var url, tail;
+    else if ((nodeName === 'img' || nodeName === 'source') && name === 'srcset')
+      attrValueElement.appendChild(linkifySrcset.call(this, value));
+    else
+      setValueWithEntities.call(this, attrValueElement, value);
+
+    if (hasText)
+      attrSpanElement.createTextChild('"');
 
+    /**
+     * @param {string} value
+     * @return {!DocumentFragment}
+     * @this {!Elements.ElementsTreeElement}
+     */
+    function linkifySrcset(value) {
+      // Splitting normally on commas or spaces will break on valid srcsets "foo 1x,bar 2x" and "data:,foo 1x".
+      // 1) Let the index of the next space be `i`.
+      // 2a) If the character at `i-1` is a comma, collect the preceding characters up to `i-1` as a URL and repeat 1).
+      // 2b) Else, collect the preceding characters as a URL.
+      // 3) Collect the characters from `i` up to the next comma as the size descriptor.
+      // https://html.spec.whatwg.org/multipage/embedded-content.html#parse-a-srcset-attribute
+      var fragment = createDocumentFragment();
+      var i = 0;
+      while (value.length) {
+        if (i++ > 0)
+          fragment.createTextChild(' ');
+        value = value.trim();
+        var url = '';
+        var descriptor = '';
+        var indexOfSpace = value.search(/\s/);

[dgozman, 2017/05/08 23:37:40]

Comment above calls this 'i'. Let's align

[luoe, 2017/05/09 02:53:18]

Done.

         if (indexOfSpace === -1) {
-          url = source;
+          url = value;
+        } else if (indexOfSpace > 0 && value[indexOfSpace - 1] === ',') {
+          url = value.substring(0, indexOfSpace);
         } else {
-          url = source.substring(0, indexOfSpace);
-          tail = source.substring(indexOfSpace);
+          url = value.substring(0, indexOfSpace);
+          var indexOfComma = value.indexOf(',', indexOfSpace);
+          if (indexOfComma !== -1)
+            descriptor = value.substring(indexOfSpace, indexOfComma + 1);

[dgozman, 2017/05/08 23:37:40]

Should this be indexOfComma-1 ?

[luoe, 2017/05/09 02:53:18]

No, I actually want to include the comma in the `descriptor`.

"foo 1x,bar 2x"
=> url: "foo", descriptor: " 1x,"

+          else
+            descriptor = value.substring(indexOfSpace);
         }
 
-        attrValueElement.appendChild(linkifyValue.call(this, url));
-
-        if (tail)
-          attrValueElement.createTextChild(tail);
+        if (url)
+          fragment.appendChild(linkifyValue.call(this, url));
+        if (descriptor)
+          fragment.createTextChild(descriptor);
+        value = value.substring(url.length + descriptor.length);
       }
-    } else {
-      setValueWithEntities.call(this, attrValueElement, value);
+      return fragment;
     }
-
-    if (hasText)
-      attrSpanElement.createTextChild('"');
   }
 
   /**