Move compareVersion function into separate module and export

[kzar]

Background

To implement a workaround for #7430 I would like to emit an event, but only for users running Firefox versions 66 or 67. To do that I need a way to compare info.platformVersion to those numbers. This logic has already been implemented in adblockpluscore/lib/notification.js, but is not exported for use elsewhere.

What to change

• Move the compareVersion function into its own module lib/versions.js, rename it to compareVersions and export.
• Add unit tests for the compareVersions function.
• Import that function from lib/notification.js and adjust the calling code to use the new name.

Hints for testers

Set up a notification.json on your test server:

{
  "notifications": [
    {
      "type": "critical",
      "title": "Hello",
      "message": "I just want to say hello.",
      "targets": [
        {
          "applicationMaxVersion": "75.0.3759.4"
        }
      ]
    }
  ],
  "version": "201904101149"
}

(Note: I'm not sure how you normally test notifications, but I imagine you redirect the request to https://notification.adblockplus.org/notification.json to your own server somehow.)

The value of applicationMaxVersion should be set to be greater than or equal to the version number you see in the About dialog of your browser.

Now do a fresh install of the extension, wait for a minute, and see that you get notified with the message I just want to say hello.

Next, edit the notification.json to make the value of applicationMaxVersion less than your browser's version number; then do a fresh install of the extension again, wait for a minute, and see that you do not get notified with the same message.

This should be enough, but you could also experiment with applicationMinVersion in the same way if you like, try different values (less than the browser version, greater than the browser version, exactly equal to the browser version), values with only the major version number, values with only the major and minor version numbers, and so on.

It would be a good idea to test this on all supported platforms since the formats of the version numbers may differ.

[mjethani]

I wouldn't mind if this went into its own lib/versions.js module with an accompanying test/versions.js test file.

While we're at it, maybe it's a good opportunity to rename the function to compareVersions() since it actually compares two versions against each other.

[kzar]

Sounds good to me, updated the description.

[sebastian]

Note that the compareVersion() function were previously part of the public API, but we stopped exporting it, in order to simplify things. I don't think that at the time we need it for anything outside of the notifications module.

For the workaround mentioned (#7430) where we want to target only specific Firefox versions, we will be just using parseInt() which is good enough when only checking for the major version.

[mjethani]

Does this mean that #7430 does not depend on this change?

[kzar]

Yea, since we could go ahead with #7430 without this change I didn't mark this issue as blocking it and I added the keyword "goodfirstbug" in case a new contributor wanted to pick it up.

Anyway, you already did a pretty good job of exporting the function and I'm in favour of the change. Even if we don't use it for #7430 it's the kind of function that will surely be useful in the future IMO. Even better now that you've added a bunch of great tests too, surely this is a good thing?

[mjethani]

Yes, I agree it's a good idea to move the function to its own module and add some tests. For reference, we might even need it for snippets (#7451).

[abpbot]

A commit referencing this issue has landed:
​Issue 7449 - Move compareVersion() to lib/versions.js

[Ross]

Done. Working as described. Tested with a variety of targets and version numbers.

ABP 0.9.15.2339
Microsoft Edge 44.17763.1.0 / Windows 10 1809

ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809