Update adblockpluscore dependency to hg:cd837fd7d60e

[mjethani]

Background

There was a regression involving CSP filters (#7043). It has been fixed in the next branch in core, but now we need to update the dependency in adblockpluschrome.

The dependency should be updated to hg:cd837fd7d60e git:98b61a8. This would include one change to a comment in lib/content/snippets.js (to fix ESLint errors), one change to a unit test, and the change for #7043.

What to change

Update adblockpluscore dependency to hg:cd837fd7d60e git:98b61a8.

Hints for testers

Ensure that whitelisting filters with the $csp filter option work as expected, see the testing hints of issue 7043 for more information.

[mjethani]

Dave, if you agree about including the fix for #7043 then I can prepare a patch.

[jsonesen]

I have opened a review for this:

​https://codereview.adblockplus.org/29907589/

[mjethani]

Replying to jsonesen:

I have opened a review for this:

​https://codereview.adblockplus.org/29907589/

That is for the next branch of adblockpluschrome (targeting the next release). This issue and the dependency update for it is for the master branch, targeting Adblock Plus 3.4.

I have not cherry-picked the commit in core from the next into the master branch, but if Dave agrees I will do it and update the issue title/description here.

[jsonesen]

Replying to mjethani:

That is for the next branch of adblockpluschrome (targeting the next release). This issue and the dependency update for it is for the master branch, targeting Adblock Plus 3.4.

Ah dang, my bad I was confused and in thinking about the next update overlooked that this is for 3.4 so thanks for explaining.

[kzar]

Replying to mjethani:

I have not cherry-picked the commit in core from the next into the master branch, but if Dave agrees I will do it and update the issue title/description here.

Yes, I agree that's what we should do. It's a regression since the last release, so the fix should be included. But like you say, we should cherry pick the fix to avoid including any other unnecessary changes into the upcoming release.

[kzar]

Heads up Ross, we're going to include a fix for the $csp whitelisting filter regression in the upcoming release.

[mjethani]

Replying to mjethani:

The dependency should be updated to hg:cd837fd7d60e git:98b61a8. This would include one change to a comment in lib/content/snippets.js (to fix ESLint errors), one change to a unit test, and the change for #7043.

Dave, we had already pushed the two other changes to master in order to fix ESLint errors and Node.js warnings. These do not affect the extension, therefore the only relevant change here is #7043.

[kzar]

Replying to mjethani:

Dave, we had already pushed the two other changes to master in order to fix ESLint errors and Node.js warnings.

Fine by me, so long as you verified those changes really had no effect on the generated builds. (I usually build before and after the change, initialise the former as a Git repository, then move the .git directory across to the later, then finally do git status to see what's changed.)

[mjethani]

Replying to kzar:

Replying to mjethani:

Dave, we had already pushed the two other changes to master in order to fix ESLint errors and Node.js warnings.

Fine by me, so long as you verified those changes really had no effect on the generated builds. (I usually build before and after the change, initialise the former as a Git repository, then move the .git directory across to the later, then finally do git status to see what's changed.)

That's a good idea.

So I checked and there is this extra comment in snippets.js:

<  *
<  * @returns {string} The computed CSS text.

Of course, but no other extra changes.

[abpbot]

A commit referencing this issue has landed:
​Issue 7047 - Update adblockpluscore dependency to hg:cd837fd7d60e

[Ross]

Done. The CSP changes included with ticket work as expected.

ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10