Opened 12 months ago

Last modified 12 months ago

#7047 closed change

Update adblockpluscore dependency to hg:cd837fd7d60e — at Version 11

Reported by: mjethani Assignee:
Priority: P1 Milestone: Adblock-Plus-3.4-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: sebastian, kzar, jsonesen, Ross Blocked By: #7043
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29912585/

Description (last modified by kzar)

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.

Change History (11)

comment:1 Changed 12 months ago by mjethani

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

comment:2 follow-up: Changed 12 months ago by jsonesen

I have opened a review for this:

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

comment:3 in reply to: ↑ 2 ; follow-ups: Changed 12 months ago by 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.

comment:4 in reply to: ↑ 3 Changed 12 months ago by 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.

comment:5 in reply to: ↑ 3 Changed 12 months ago by 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.

comment:6 Changed 12 months ago by kzar

  • Cc Ross added
  • Milestone set to Adblock-Plus-3.4-for-Chrome-Opera-Firefox
  • Priority changed from Unknown to P1

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

comment:7 Changed 12 months ago by mjethani

  • Description modified (diff)
  • Summary changed from Update adblockpluscore dependency to [TBD: for issue 7043] to Update adblockpluscore dependency to hg:cd837fd7d60e

comment:8 Changed 12 months ago by mjethani

  • Description modified (diff)

comment:9 in reply to: ↑ description ; follow-up: Changed 12 months ago by 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.

comment:10 in reply to: ↑ 9 Changed 12 months ago by 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.)

comment:11 Changed 12 months ago by kzar

  • Description modified (diff)
  • Ready set
Note: See TracTickets for help on using tickets.