DevTools: correctly parse srcset attribute before linkifying urls

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

 /*
  * Copyright (C) 2007, 2008 Apple Inc.  All rights reserved.
  * Copyright (C) 2008 Matt Lilek <webkit@mattlilek.com>
  * Copyright (C) 2009 Joseph Pecoraro
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
@@ skipping 1263 matching lines @@
       value = value.replace(closingPunctuationRegex, '$&\u200B');
       if (value.startsWith('data:'))
         value = value.trimMiddle(60);
       var link = node.nodeName().toLowerCase() === 'a' ?
           UI.createExternalLink(rewrittenHref, value, '', true) :
           Components.Linkifier.linkifyURL(rewrittenHref, value, '', undefined, undefined, true);
       link[Elements.ElementsTreeElement.HrefSymbol] = rewrittenHref;
       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;
-
-        if (indexOfSpace === -1) {
-          url = source;
-        } else {
-          url = source.substring(0, indexOfSpace);
-          tail = source.substring(indexOfSpace);
-        }
-
-        attrValueElement.appendChild(linkifyValue.call(this, url));
-
-        if (tail)
-          attrValueElement.createTextChild(tail);
-      }
-    } else {
+    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 = value;
+        } else if (indexOfSpace > 0 && value[indexOfSpace - 1] === ',') {
+          url = value.substring(0, indexOfSpace);
+        } else {
+          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);
+        }
+
+        if (url)
+          fragment.appendChild(linkifyValue.call(this, url));
+        if (descriptor)
+          fragment.createTextChild(descriptor);
+        value = value.substring(url.length + descriptor.length);
+      }
+      return fragment;
+    }
   }
 
   /**
    * @param {!Node} parentElement
    * @param {string} pseudoElementName
    */
   _buildPseudoElementDOM(parentElement, pseudoElementName) {
     var pseudoElement = parentElement.createChild('span', 'webkit-html-pseudo-element');
     pseudoElement.textContent = '::' + pseudoElementName;
     parentElement.createTextChild('\u200B');
@@ skipping 305 matching lines @@
 Elements.ElementsTreeElement.ForbiddenClosingTagElements = new Set([
   'area', 'base',  'basefont', 'br',   'canvas',   'col',  'command', 'embed',  'frame', 'hr',
   'img',  'input', 'keygen',   'link', 'menuitem', 'meta', 'param',   'source', 'track', 'wbr'
 ]);
 
 // These tags we do not allow editing their tag name.
 Elements.ElementsTreeElement.EditTagBlacklist = new Set(['html', 'head', 'body']);
 
 /** @typedef {{cancel: function(), commit: function(), resize: function(), editor:!UI.TextEditor}} */
 Elements.MultilineEditorController;