Issue 1801 - Use URL objects to process URLs in the background page

lib/url.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 55 matching lines @@
 /**
  * Gets the IDN-decoded hostname from the URL of a frame.
  * If the URL don't have host information (like "about:blank"
  * and "data:" URLs) it falls back to the parent frame.
  *
  * @param {Frame} [frame]
  * @return {string}
  */
 function extractHostFromFrame(frame)
 {
   for (; frame; frame = frame.parent)

[kzar, 2015/01/26 10:27:38]

Took me a second to grok this loop, wouldn't a while loop be clearer here?

while (frame)
{
  if (frame.url)
  {
    let hostname = getDecodedHostname(frame.url);
    if (hostname)
      return hostname;
  }

  frame = frame.parent;
}

return "";

[Sebastian Noack, 2015/01/26 10:38:39]

I think this a typical case for-lops exist for, to check for a condition and
advance the state in the header. This isn't necessarily limited to counters.

[kzar, 2015/01/26 17:19:05]

Hmm well I guess it's subjective which form is easier to read. I'll leave the
deciding vote to Wladimir :D

[Sebastian Noack, 2015/01/26 17:26:31]

As long as you don't consider for-loops generally less readable than while-loops
like below, there is objectively no reason why you shouldn't use for-loops here.

var i = 0;
while (i < 10)
{
  ...
  i++;
}

[Wladimir Palant, 2015/02/11 13:45:45]

I think I wrote the for loop in the original code, it went exactly like this one
:)

In general, I do like using for loops wherever it makes sense - meaning that the
increment doesn't have to be trivial. I would probably have concerns if it were
some complicated construct, but this one isn't.

   {
     let hostname = getDecodedHostname(frame.url);
     if (hostname)
       return hostname;
   }
 
   return "";
 }
 exports.extractHostFromFrame = extractHostFromFrame;
 
@@ skipping 12 matching lines @@
 
   let host = getDecodedHostname(url);
   if (url.port)
     host += ":" + url.port;
   return protocol + "//" + host + url.pathname + url.search;
 }
 exports.stringifyURL = stringifyURL;
 
 function isDomain(hostname)
 {
-  return /[^\d.]/.test(hostname) && hostname.indexOf(":") == -1;

[Wladimir Palant, 2015/02/09 12:54:29]

new URL("http://0x5bec7a38/").hostname - gives you "0x5bec7a38" in Safari.
Chrome normalizes IPv4 addresses however. There is also
"http://0x5b.236.122.56/" by the way.

Also, please add a comment - it's not obvious what you are testing here.

[Sebastian Noack, 2015/02/11 10:55:51]

Done.

+  // No hostname or IPv4 address, also considering hexadecimal octets.
+  if (/^((0x[\da-f]+|\d+)(\.|$))*$/i.test(hostname))
+    return false;
+
+  // IPv6 address. Since there can't be colons in domains, we can
+  // just check whether there are any colons to exclude IPv6 addresses.
+  return hostname.indexOf(":") == -1;
 }
 
 function getBaseDomain(hostname)
 {
   let bits = hostname.split(".");
   let cutoff = bits.length - 2;
 
   for (let i = 0; i < bits.length; i++)
   {
     let offset = publicSuffixes[bits.slice(i).join(".")];
@@ skipping 25 matching lines @@
 
   if (requestHost == documentHost)
     return false;
 
   if (!isDomain(requestHost) || !isDomain(documentHost))
     return true;
 
   return getBaseDomain(requestHost) != getBaseDomain(documentHost);
 }
 exports.isThirdParty = isThirdParty;