Noissue - Removed redundant URL compatibility logic

Noissue - Removed redundant URL compatibility logic

[Sebastian Noack, 2016-04-04 11:19:46]

https://codereview.adblockplus.org/29339343/diff/29339344/lib/notification.js
File lib/notification.js (right):

https://codereview.adblockplus.org/29339343/diff/29339344/lib/notification.js...
lib/notification.js:222: let uri = Utils.makeURI(url);
Utils.makeURI() wraps Services.io.newURI() and returns null if an exception is
raised. Moreover, as it's also used by the synchronizer, it therefore already
has implementations for other platforms like Chrome.

[Wladimir Palant, 2016-04-04 12:57:36]

https://codereview.adblockplus.org/29339343/diff/29339344/lib/notification.js
File lib/notification.js (right):

https://codereview.adblockplus.org/29339343/diff/29339344/lib/notification.js...
lib/notification.js:222: let uri = Utils.makeURI(url);
On 2016/04/04 11:19:46, Sebastian Noack wrote:
> Utils.makeURI() wraps Services.io.newURI() and returns null if an exception is
> raised. Moreover, as it's also used by the synchronizer, it therefore already
> has implementations for other platforms like Chrome.

Actually, it is better to use `new URL` unconditionally - it has been added in
https://issues.adblockplus.org/ticket/3834 and can be expected to exist on
Firefox as well.

[Sebastian Noack, 2016-04-04 13:40:38]

https://codereview.adblockplus.org/29339343/diff/29339344/lib/notification.js
File lib/notification.js (right):

https://codereview.adblockplus.org/29339343/diff/29339344/lib/notification.js...
lib/notification.js:222: let uri = Utils.makeURI(url);
On 2016/04/04 12:57:36, Wladimir Palant wrote:
> On 2016/04/04 11:19:46, Sebastian Noack wrote:
> > Utils.makeURI() wraps Services.io.newURI() and returns null if an exception
is
> > raised. Moreover, as it's also used by the synchronizer, it therefore
already
> > has implementations for other platforms like Chrome.
> 
> Actually, it is better to use `new URL` unconditionally - it has been added in
> https://issues.adblockplus.org/ticket/3834 and can be expected to exist on
> Firefox as well.

Awesome. New patch set is up.

[Wladimir Palant, 2016-04-04 13:49:13]

LGTM, and I hope that you can drop Utils.makeURI in Chrome now.

[Sebastian Noack, 2016-04-04 13:52:25]

On 2016/04/04 13:49:13, Wladimir Palant wrote:
> LGTM, and I hope that you can drop Utils.makeURI in Chrome now.

Yes, that is the plan. And since these were the only occurrences, we can do that
now.

[Sebastian Noack, 2016-04-04 14:05:40]

https://codereview.adblockplus.org/29339343/diff/29339372/lib/notification.js
File lib/notification.js (right):

https://codereview.adblockplus.org/29339343/diff/29339372/lib/notification.js...
lib/notification.js:225: host = new URL(url).hostname;
Sorry, I missed that. Before we used the hostname not the full host.

[Wladimir Palant, 2016-04-04 14:37:22]

LGTM