Issue 4795 - Use modern JavaScript syntax

Issue 4795 - Use modern JavaScript syntax

[kzar, 2017-01-13 12:12:52]

Patch Set 1

(I've quickly tested basic functionality but need to test these changes more
carefully tomorrow.)

[Sebastian Noack, 2017-01-13 12:30:50]

https://codereview.adblockplus.org/29371763/diff/29371764/chrome/ext/common.js
File chrome/ext/common.js (left):

https://codereview.adblockplus.org/29371763/diff/29371764/chrome/ext/common.j...
chrome/ext/common.js:18: (function()
As mentioned on the other review, unless you use strict mode, named functions
are still defined globally without IFEE. There are no named functions in this
file, but while on it, why not just add "use strict" everywhere as well? This is
arguably any more risky than the changes you are doing here anyway. And that way
we don't potentially export any named functions (in the future), plus we benefit
from other advantages of strict mode.

https://codereview.adblockplus.org/29371763/diff/29371764/chrome/ext/common.js
File chrome/ext/common.js (right):

https://codereview.adblockplus.org/29371763/diff/29371764/chrome/ext/common.j...
chrome/ext/common.js:28: getWindow: () => chrome.extension.getBackgroundPage()
I think for method definitions, the method definition shorthand syntax is
preferable over arrow functions:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/M...

The only question is whether it is supported by Microsoft Edge, yet?

[kzar, 2017-01-16 04:09:12]

Patch Set 2 : "use strict";
Patch Set 3 : Use method shorthand

https://codereview.adblockplus.org/29371763/diff/29371764/chrome/ext/common.js
File chrome/ext/common.js (left):

https://codereview.adblockplus.org/29371763/diff/29371764/chrome/ext/common.j...
chrome/ext/common.js:18: (function()
On 2017/01/13 12:30:50, Sebastian Noack wrote:
> As mentioned on the other review, unless you use strict mode, named functions
> are still defined globally without IFEE. There are no named functions in this
> file, but while on it, why not just add "use strict" everywhere as well? This
is
> arguably any more risky than the changes you are doing here anyway. And that
way
> we don't potentially export any named functions (in the future), plus we
benefit
> from other advantages of strict mode.

Done.

https://codereview.adblockplus.org/29371763/diff/29371764/chrome/ext/common.js
File chrome/ext/common.js (right):

https://codereview.adblockplus.org/29371763/diff/29371764/chrome/ext/common.j...
chrome/ext/common.js:28: getWindow: () => chrome.extension.getBackgroundPage()
On 2017/01/13 12:30:50, Sebastian Noack wrote:
> I think for method definitions, the method definition shorthand syntax is
> preferable over arrow functions:
>
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/M...
> 
> The only question is whether it is supported by Microsoft Edge, yet?

According to this page[1] they are. Also I just tested them in Edge 38 myself
and they seem to work OK. Done.

[1] - http://kangax.github.io/compat-table/es5/

[kzar, 2017-01-16 08:48:09]

Patch Set 4 : Workaround limitation with iterating element collections in older
versions of Chrome

https://codereview.adblockplus.org/29371763/diff/29371954/lib/notificationHel...
File lib/notificationHelper.js (right):

https://codereview.adblockplus.org/29371763/diff/29371954/lib/notificationHel...
lib/notificationHelper.js:38: let canUseChromeNotifications = typeof chrome !=
"undefined" && "notifications" in chrome;
(Due to rebase.)

[Sebastian Noack, 2017-01-16 14:47:41]

https://codereview.adblockplus.org/29371763/diff/29371998/chrome/ext/common.js
File chrome/ext/common.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/chrome/ext/common.j...
chrome/ext/common.js:38: getWindow() { return
chrome.extension.getBackgroundPage(); }
Turning this into a one-liner seems unrelated. I'm also undecided whether
compressing a function (except for arrow functions) into a single line makes the
the code harder to read. As for consistency, I think we generally avoid that.

https://codereview.adblockplus.org/29371763/diff/29371998/ext/background.js
File ext/background.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/ext/background.js#n...
ext/background.js:21: let nonEmptyPageMaps = Object.create(null);
In "modern JavaScript" you wouldn't (mis)use objects as hashtable, but use Map
objects instead. Not sure whether it is worth to change this, if it is only for
the to be removed abstraction layer. But there might be more occurrences of this
pattern.

https://codereview.adblockplus.org/29371763/diff/29371998/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/include.preload.js#...
include.preload.js:276: for (let i = 0; i < this.changedNodes.length; i++)
Wouldn't this cause the the variable i form the outer scope to be overridden?

[kzar, 2017-01-16 14:59:11]

Patch Set 6 : Undo accidental whitespace change

https://codereview.adblockplus.org/29371763/diff/29371998/chrome/ext/common.js
File chrome/ext/common.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/chrome/ext/common.j...
chrome/ext/common.js:38: getWindow() { return
chrome.extension.getBackgroundPage(); }
On 2017/01/16 14:47:40, Sebastian Noack wrote:
> Turning this into a one-liner seems unrelated. I'm also undecided whether
> compressing a function (except for arrow functions) into a single line makes
the
> the code harder to read. As for consistency, I think we generally avoid that.

Done.

https://codereview.adblockplus.org/29371763/diff/29371998/ext/background.js
File ext/background.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/ext/background.js#n...
ext/background.js:21: let nonEmptyPageMaps = Object.create(null);
On 2017/01/16 14:47:40, Sebastian Noack wrote:
> In "modern JavaScript" you wouldn't (mis)use objects as hashtable, but use Map
> objects instead. Not sure whether it is worth to change this, if it is only
for
> the to be removed abstraction layer. But there might be more occurrences of
this
> pattern.

Good point, but would you mind if we left that for a second review? This is
already getting fairly unwieldly.

https://codereview.adblockplus.org/29371763/diff/29371998/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/include.preload.js#...
include.preload.js:276: for (let i = 0; i < this.changedNodes.length; i++)
On 2017/01/16 14:47:40, Sebastian Noack wrote:
> Wouldn't this cause the the variable i form the outer scope to be overridden?

I don't think that there is a variable i from the outer scope here, or do I miss
something?

[kzar, 2017-01-16 15:00:48]

https://codereview.adblockplus.org/29371763/diff/29371998/lib/notificationHel...
File lib/notificationHelper.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/lib/notificationHel...
lib/notificationHelper.js:38: let canUseChromeNotifications = typeof chrome !=
"undefined" && "notifications" in chrome;
(Due to rebase again.)

[Sebastian Noack, 2017-01-16 15:35:58]

https://codereview.adblockplus.org/29371763/diff/29371998/ext/background.js
File ext/background.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/ext/background.js#n...
ext/background.js:21: let nonEmptyPageMaps = Object.create(null);
On 2017/01/16 14:59:07, kzar wrote:
> On 2017/01/16 14:47:40, Sebastian Noack wrote:
> > In "modern JavaScript" you wouldn't (mis)use objects as hashtable, but use
Map
> > objects instead. Not sure whether it is worth to change this, if it is only
> for
> > the to be removed abstraction layer. But there might be more occurrences of
> this
> > pattern.
> 
> Good point, but would you mind if we left that for a second review? This is
> already getting fairly unwieldly.

Acknowledged.

https://codereview.adblockplus.org/29371763/diff/29371998/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/include.preload.js#...
include.preload.js:276: for (let i = 0; i < this.changedNodes.length; i++)
On 2017/01/16 14:59:07, kzar wrote:
> On 2017/01/16 14:47:40, Sebastian Noack wrote:
> > Wouldn't this cause the the variable i form the outer scope to be
overridden?
> 
> I don't think that there is a variable i from the outer scope here, or do I
miss
> something?

Never mind, the outer loop is now using for-of, and the loop above that is also
using the variable i is now block scoped. I missed that.

https://codereview.adblockplus.org/29371763/diff/29372051/chrome/ext/content.js
File chrome/ext/content.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/chrome/ext/content....
chrome/ext/content.js:8: ext.onExtensionUnloaded = (() =>
The IFEE here existed originally to encapsulate the underlying port without
exporting it. As for the other IFEEs, the same can be achieved without, thanks
to block-scoping:

  {
    let port = null;

    ext.onExtensionUnloaded = {
      ...
    };
  }

https://codereview.adblockplus.org/29371763/diff/29372051/chrome/ext/popup.js
File chrome/ext/popup.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/chrome/ext/popup.js...
chrome/ext/popup.js:17: getWindow() { return backgroundPage; }
As pointed out in a different file, in the previous review round, these
whitespace changes are unrelated, and are arguably desirable.

https://codereview.adblockplus.org/29371763/diff/29372051/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/include.preload.js#...
include.preload.js:84: for (let child of element.children)
Removing the braces for the loop is unrelated, and according to our coding
practices, we don't omit lines for outer blocks, which contain more than one
line, anyway.

https://codereview.adblockplus.org/29371763/diff/29372051/lib/utils.js
File lib/utils.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/lib/utils.js#newcode22
lib/utils.js:22: getString(id) { return ext.i18n.getMessage("global_" + id); },
Again, unrelated whitespace changes.

https://codereview.adblockplus.org/29371763/diff/29372051/lib/utils.js#newcode48
lib/utils.js:48: generateChecksum(lines) { return null; },
Again, unrelated whitespace changes.

https://codereview.adblockplus.org/29371763/diff/29372051/notification.js
File notification.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/notification.js#new...
notification.js:23: let Utils = require("utils").Utils;
I suppose we could better use destructuring here now?

https://codereview.adblockplus.org/29371763/diff/29372051/popup.js
File popup.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/popup.js#newcode23
popup.js:23: let Filter = require("filterClasses").Filter;
I suppose we could better use destructuring here now?

https://codereview.adblockplus.org/29371763/diff/29372051/qunit/tests/cssEsca...
File qunit/tests/cssEscaping.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/qunit/tests/cssEsca...
qunit/tests/cssEscaping.js:4: let filterClasses = require("filterClasses");
I suppose we could better use destructuring here now?

https://codereview.adblockplus.org/29371763/diff/29372051/qunit/tests/url.js
File qunit/tests/url.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/qunit/tests/url.js#...
qunit/tests/url.js:21: let url = require("url");
I suppose we could better use destructuring here now?

[Sebastian Noack, 2017-01-16 15:37:24]

https://codereview.adblockplus.org/29371763/diff/29372051/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/include.preload.js#...
include.preload.js:84: for (let child of element.children)
On 2017/01/16 15:35:55, Sebastian Noack wrote:
> Removing the braces for the loop is unrelated, and according to our coding
> practices, we don't omit lines for outer blocks, which contain more than one
> line, anyway.

s/omit lines/omit braces/

[kzar, 2017-01-17 07:42:51]

Patch Set 7 : Addressed feedback, used destructuring

https://codereview.adblockplus.org/29371763/diff/29371998/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29371763/diff/29371998/include.preload.js#...
include.preload.js:276: for (let i = 0; i < this.changedNodes.length; i++)
On 2017/01/16 15:35:54, Sebastian Noack wrote:
> On 2017/01/16 14:59:07, kzar wrote:
> > On 2017/01/16 14:47:40, Sebastian Noack wrote:
> > > Wouldn't this cause the the variable i form the outer scope to be
> overridden?
> > 
> > I don't think that there is a variable i from the outer scope here, or do I
> miss
> > something?
> 
> Never mind, the outer loop is now using for-of, and the loop above that is
also
> using the variable i is now block scoped. I missed that.

Acknowledged.

https://codereview.adblockplus.org/29371763/diff/29372051/chrome/ext/content.js
File chrome/ext/content.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/chrome/ext/content....
chrome/ext/content.js:8: ext.onExtensionUnloaded = (() =>
On 2017/01/16 15:35:55, Sebastian Noack wrote:
> The IFEE here existed originally to encapsulate the underlying port without
> exporting it. As for the other IFEEs, the same can be achieved without, thanks
> to block-scoping:
> 
>   {
>     let port = null;
> 
>     ext.onExtensionUnloaded = {
>       ...
>     };
>   }

Done.

https://codereview.adblockplus.org/29371763/diff/29372051/chrome/ext/popup.js
File chrome/ext/popup.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/chrome/ext/popup.js...
chrome/ext/popup.js:17: getWindow() { return backgroundPage; }
On 2017/01/16 15:35:55, Sebastian Noack wrote:
> As pointed out in a different file, in the previous review round, these
> whitespace changes are unrelated, and are arguably desirable.

Whoops, another victim from switching to an inline arrow function and back to
the new method definition syntax. Done.

https://codereview.adblockplus.org/29371763/diff/29372051/include.preload.js
File include.preload.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/include.preload.js#...
include.preload.js:84: for (let child of element.children)
On 2017/01/16 15:37:24, Sebastian Noack wrote:
> On 2017/01/16 15:35:55, Sebastian Noack wrote:
> > Removing the braces for the loop is unrelated, and according to our coding
> > practices, we don't omit lines for outer blocks, which contain more than one
> > line, anyway.
> 
> s/omit lines/omit braces/

Whoops, Done.

https://codereview.adblockplus.org/29371763/diff/29372051/lib/utils.js
File lib/utils.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/lib/utils.js#newcode22
lib/utils.js:22: getString(id) { return ext.i18n.getMessage("global_" + id); },
On 2017/01/16 15:35:55, Sebastian Noack wrote:
> Again, unrelated whitespace changes.

Done.

https://codereview.adblockplus.org/29371763/diff/29372051/lib/utils.js#newcode48
lib/utils.js:48: generateChecksum(lines) { return null; },
On 2017/01/16 15:35:56, Sebastian Noack wrote:
> Again, unrelated whitespace changes.

Done.

https://codereview.adblockplus.org/29371763/diff/29372051/notification.js
File notification.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/notification.js#new...
notification.js:23: let Utils = require("utils").Utils;
On 2017/01/16 15:35:56, Sebastian Noack wrote:
> I suppose we could better use destructuring here now?

Done.

https://codereview.adblockplus.org/29371763/diff/29372051/popup.js
File popup.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/popup.js#newcode23
popup.js:23: let Filter = require("filterClasses").Filter;
On 2017/01/16 15:35:56, Sebastian Noack wrote:
> I suppose we could better use destructuring here now?

Done.

https://codereview.adblockplus.org/29371763/diff/29372051/qunit/tests/cssEsca...
File qunit/tests/cssEscaping.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/qunit/tests/cssEsca...
qunit/tests/cssEscaping.js:4: let filterClasses = require("filterClasses");
On 2017/01/16 15:35:57, Sebastian Noack wrote:
> I suppose we could better use destructuring here now?

Done.

https://codereview.adblockplus.org/29371763/diff/29372051/qunit/tests/url.js
File qunit/tests/url.js (right):

https://codereview.adblockplus.org/29371763/diff/29372051/qunit/tests/url.js#...
qunit/tests/url.js:21: let url = require("url");
On 2017/01/16 15:35:57, Sebastian Noack wrote:
> I suppose we could better use destructuring here now?

Done.

[Sebastian Noack, 2017-01-17 17:18:35]

Dependent on the outcome of the discussion regarding the usage of const (for
module imports) in the other review [1], that might effect this patch as well.
Otherwise this change looks good to me now.

[1]:
https://codereview.adblockplus.org/29370999/diff/29371943/messageResponder.js...

[kzar, 2017-01-18 07:36:41]

Patch Set 8 : Use const as per the new rules
Patch Set 9 : Define and destructure backgroundPage more consistently, fix minor
whitespace errors

[Sebastian Noack, 2017-01-18 11:24:32]

https://codereview.adblockplus.org/29371763/diff/29372355/chrome/ext/popup.js
File chrome/ext/popup.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/chrome/ext/popup.js...
chrome/ext/popup.js:5: window.ext = Object.create(backgroundPage.ext);
Assigning to the global object (i.e. window) isn't necessary anymore. Using var
seems more appropriate here. However, this wasn't possible before because this
code was wrapped by an IFEE. Feel free to grab for window through the repository
to find more cases like that.

https://codereview.adblockplus.org/29371763/diff/29372355/lib/notificationHel...
File lib/notificationHelper.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/lib/notificationHel...
lib/notificationHelper.js:67: const regex = /<a>(.*?)<\/a>/g;
These is one of the scenarios where I'm undecided whether using const would go
too far. Also note that the rule we came up with doesn't target this case, as
the expression could not be inlined, as the exec() call advances the lastIndex
of the RegExp object.

https://codereview.adblockplus.org/29371763/diff/29372355/lib/utils.js
File lib/utils.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/lib/utils.js#newcode50
lib/utils.js:50: // We cannot calculate MD5 checksums yet :-(
Moving the comment here seems unrelated.

https://codereview.adblockplus.org/29371763/diff/29372355/lib/utils.js#newcode77
lib/utils.js:77: let prefix =
require("utils").Utils.checkLocalePrefixMatch(subscription.getAttribute("prefixes"));
Is require("utils") even necessary? Couldn't we just access Utils directly?

https://codereview.adblockplus.org/29371763/diff/29372355/lib/utils.js#newcod...
lib/utils.js:107: const {Prefs} = require("prefs");
In this case, perhaps just inline it:

  let docLink = require('prefs').Prefs.documentation_link;

https://codereview.adblockplus.org/29371763/diff/29372355/notification.js
File notification.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/notification.js#new...
notification.js:21: const {require} = backgroundPage;
Note that backgroundPage isn't used anywhere else, so how about this:

  const {require} = ext.backgroundPage.getWindow();

https://codereview.adblockplus.org/29371763/diff/29372355/popup.js
File popup.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/popup.js#newcode21
popup.js:21: const {require} = backgroundPage;
Note that backgroundPage isn't used anywhere else, so how about this:

  const {require} = ext.backgroundPage.getWindow();

https://codereview.adblockplus.org/29371763/diff/29372355/stats.js
File stats.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/stats.js#newcode22
stats.js:22: const {require} = backgroundPage;
Note that backgroundPage isn't used anywhere else, so how about this:

  const {require} = ext.backgroundPage.getWindow();

https://codereview.adblockplus.org/29371763/diff/29372355/utils.js
File utils.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/utils.js#newcode4
utils.js:4: const {require, Services} = backgroundPage;
Note that backgroundPage isn't used anywhere else, so how about this:

  const {require, Services} = ext.backgroundPage.getWindow();

[kzar, 2017-01-18 11:46:43]

Patch Set 10 : Addressed some more feedback

https://codereview.adblockplus.org/29371763/diff/29372355/chrome/ext/popup.js
File chrome/ext/popup.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/chrome/ext/popup.js...
chrome/ext/popup.js:5: window.ext = Object.create(backgroundPage.ext);
On 2017/01/18 11:24:26, Sebastian Noack wrote:
> Assigning to the global object (i.e. window) isn't necessary anymore. Using
var
> seems more appropriate here. However, this wasn't possible before because this
> code was wrapped by an IFEE. Feel free to grab for window through the
repository
> to find more cases like that.

Done.

https://codereview.adblockplus.org/29371763/diff/29372355/lib/notificationHel...
File lib/notificationHelper.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/lib/notificationHel...
lib/notificationHelper.js:67: const regex = /<a>(.*?)<\/a>/g;
On 2017/01/18 11:24:26, Sebastian Noack wrote:
> These is one of the scenarios where I'm undecided whether using const would go
> too far. Also note that the rule we came up with doesn't target this case, as
> the expression could not be inlined, as the exec() call advances the lastIndex
> of the RegExp object.

Let's er on the side of caution for now.

https://codereview.adblockplus.org/29371763/diff/29372355/lib/utils.js
File lib/utils.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/lib/utils.js#newcode50
lib/utils.js:50: // We cannot calculate MD5 checksums yet :-(
On 2017/01/18 11:24:27, Sebastian Noack wrote:
> Moving the comment here seems unrelated.

Whoops, well spotted. Done.

https://codereview.adblockplus.org/29371763/diff/29372355/lib/utils.js#newcode77
lib/utils.js:77: let prefix =
require("utils").Utils.checkLocalePrefixMatch(subscription.getAttribute("prefixes"));
On 2017/01/18 11:24:27, Sebastian Noack wrote:
> Is require("utils") even necessary? Couldn't we just access Utils directly?

Yes I can't see why not, Done.

https://codereview.adblockplus.org/29371763/diff/29372355/lib/utils.js#newcod...
lib/utils.js:107: const {Prefs} = require("prefs");
On 2017/01/18 11:24:28, Sebastian Noack wrote:
> In this case, perhaps just inline it:
> 
>   let docLink = require('prefs').Prefs.documentation_link;

Done.

https://codereview.adblockplus.org/29371763/diff/29372355/notification.js
File notification.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/notification.js#new...
notification.js:21: const {require} = backgroundPage;
On 2017/01/18 11:24:28, Sebastian Noack wrote:
> Note that backgroundPage isn't used anywhere else, so how about this:
> 
>   const {require} = ext.backgroundPage.getWindow();

Done.

https://codereview.adblockplus.org/29371763/diff/29372355/popup.js
File popup.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/popup.js#newcode21
popup.js:21: const {require} = backgroundPage;
On 2017/01/18 11:24:29, Sebastian Noack wrote:
> Note that backgroundPage isn't used anywhere else, so how about this:
> 
>   const {require} = ext.backgroundPage.getWindow();

Done.

https://codereview.adblockplus.org/29371763/diff/29372355/stats.js
File stats.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/stats.js#newcode22
stats.js:22: const {require} = backgroundPage;
On 2017/01/18 11:24:30, Sebastian Noack wrote:
> Note that backgroundPage isn't used anywhere else, so how about this:
> 
>   const {require} = ext.backgroundPage.getWindow();

Done.

https://codereview.adblockplus.org/29371763/diff/29372355/utils.js
File utils.js (right):

https://codereview.adblockplus.org/29371763/diff/29372355/utils.js#newcode4
utils.js:4: const {require, Services} = backgroundPage;
On 2017/01/18 11:24:30, Sebastian Noack wrote:
> Note that backgroundPage isn't used anywhere else, so how about this:
> 
>   const {require, Services} = ext.backgroundPage.getWindow();

Done.

[Sebastian Noack, 2017-01-18 12:12:27]

LGTM