Extension broken on Chrome 49

[Ross]

Environment

ABP 3.2.0.2097
Chrome 49

Works fine in Chrome 55+.
Smoke tested with Chrome 50.
Not able to check in Chrome 51-54.

Is fine in Firefox's min supported version (51)

How to reproduce

1. Install extension.
2. Attempt to open options page.

Observed behaviour

The first run page does not open. The options page cannot be opened from the popup UI. Ad blocking does not work.

Some errors from the background console:

    Uncaught SyntaxError: Invalid regular expression flags    adblockplus.js:6274

some errors from the popup console:

    extensions::uncaught_exception_handler:8 Error in event handler for (unknown): TypeError: Cannot read property 'locale' of undefined
    at browser.runtime.sendMessage (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/i18n.js:31:47)handler @ extensions::uncaught_exception_handler:8

    extensions::uncaught_exception_handler:8 Error in event handler for (unknown): TypeError: Cannot read property 'toLocaleString' of undefined
    at browser.runtime.sendMessage.blockedPage (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/popup.js:177:41)handler @ extensions::uncaught_exception_handler:8

    extensions::uncaught_exception_handler:8 Error in event handler for (unknown): TypeError: Cannot read property 'toLocaleString' of undefined
    at blockedTotal (chrome-extension://ldcecbkkoecffmfljeihcmifjjdoepkn/popup.js:184:42)

Expected behaviour

The extension to work as expected in Chrome 49.

Notes

• This is a regression since the previous release.
• This seems to only be a problem with Chrome <50.
• The problem seems to be caused by the use of the Unicode "u" regular expression flag, which ​according to MDN wasn't supported until Chrome 50. The following commits introduced the flag into our codebase:
  • ​5f851931ffea adblockpluscore/lib/snippets.js
  • ​926b9df46ce6 adblockpluscore/lib/common.js
  • ​7052e279497a adblockpluscore/test/browser/elemHideEmulation.js

[kzar]

Uncaught SyntaxError: Invalid regular expression flags adblockplus.js:6274

It seems that Chrome 49 doesn't support the "u" flag for regular expressions, this exception in particular is caused by ​5f851931ffea in adblockpluscore/lib/snippets.js, but I can see ​926b9df46ce6 added one to adblockpluscore/lib/common.js, and ​7052e279497a added one to adblockpluscore/test/browser/elemHideEmulation.js.

[kzar]

​According to MDN the Unicode flag is supported from Chrome 50 onward, but I didn't test that.

[kzar]

I can't see any obvious cause of those popup console exceptions, I suspect they might just be a side-effect of the exceptions thrown in the background console. Especially the ones that relate to the responses to messages sent to the background being undefined.

[kzar]

Ross please can you confirm if Adblock Plus 3.2 works correctly with Chrome 49?

[hfiguiere]

I think this raises the question whether we should support Chrome 49? It seems that it is because it is the last version supported by Windows XP, but then both are out of security updates. And for the other users, Google update push make it unlikely for users to not run the latest....

[Ross]

3.1.0.2069 (the last 3.1 devbuild) and 3.2.0.2087 (the first 3.2 devbuild) seem to work correctly in Chrome 49.

[kzar]

Dang alright, well I guess this is a release blocker unless we decide to drop Chrome 49 support then. Any opinion Sebastian?

[Ross]

Just to note, the testing virtual machines we use only have Chrome 49, 55, 60-68+ on them at the moment. Although if needed I could probably find a sketchy installer for 50-54 somewhere if we say bumped to one of those for min version.

[kzar]

Yea, it would be kind of useful to confirm that the extension works properly with Chrome 50 before we make a decision about dropping Chrome 49.

[Ross]

The 3.2.0.2097 devbuild appears to work in Chrome 50 after a quick check. The above console errors are not thrown and first run page, functionality appear to be working okay too.

[kzar]

Replying to Ross:

The 3.2.0.2097 devbuild appears to work in Chrome 50 after a quick check. The above console errors are not thrown and first run page, functionality appear to be working okay too.

Thanks for confirming that, I've updated the description.

[sebastian]

If there is any benefit in dropping support for Chrome 49, we can request the usage data, and look into it. But for this release, we might be moving forward faster if we just fix the incompatibility.

Also if it's just for using the u flag in regular expressions, the examples where we apparently just started to use it don't seem to make a strong case. As far as I can tell, how the u flag is used in snippet.js and common.js, it is redudnant, i.e. those regular expressions are equivalanet to the same regular expression without the u flag since they neither use unicode-escapes nor the . character. In test/browser/elemHideEmulation.js we specifically test support of regular expressions with u flag with -abp-contains(). We can either just remove this test, or skip it if the u flag isn't supported.

[kzar]

Replying to sebastian:

If there is any benefit in dropping support for Chrome 49, we can request the usage data, and look into it. But for this release, we might be moving forward faster if we just fix the incompatibility.

Also if it's just for using the u flag in regular expressions, the examples where we apparently just started to use it don't seem to make a strong case. As far as I can tell, how the u flag is used in snippet.js and common.js, it is redudnant, i.e. those regular expressions are equivalanet to the same regular expression without the u flag since they neither use unicode-escapes nor the . character. In test/browser/elemHideEmulation.js we specifically test support of regular expressions with u flag with -abp-contains(). We can either just remove this test, or skip it if the u flag isn't supported.

Manish/Hubert Please could you open a core issue to remove use of the "u" flag, or to skip its use when it isn't supported therefore? Please mark it as blocking this issue, then this issue can turn into a dependency update.

[mjethani]

My bad, indeed the u flag is useless in those cases.

Now do I understand correctly that doing another dependency update will cause the release to be delayed? I doubt that Chrome 49 has any more than 0.1% of the overall market share. It might be better instead to drop support for Chrome 49.

In any case let me open an issue for this and submit a patch.

[sebastian]

Last time (not too long ago), we looked into it, Chrome 49 had significantly more users than that. Anyway, getting updated usage data and then having a debate over it will likely take longer than just fixing this regression.

[mjethani]

#6823 is closed.

[sebastian]

Closing this issue as duplicate since there is a new issue for the dependency update that will fix this.