Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#5584 closed change (fixed)

Update adblockpluscore dependency 217eff0504a5

Reported by: hfiguiere Assignee: hfiguiere
Priority: P2 Milestone: Adblock-Plus-1.13.4-for-Chrome-Opera
Module: Platform Keywords:
Cc: fhd, trev, sebastian, kzar Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29529646/

Description (last modified by trev)

Background

Update adblockpluscore dependency

What to change

Update adblockpluscore dependency to

hg git
217eff0504a5 48eddbc

Make sure that elemHideEmulation is packaged as a module (see #5516).

Included changes


adffd8564570 Noissue - Refactored notification target checking (checkTarget) to make it more easily expandable Winsley von Spee
d84358f9e0c0 Noissue - Added testcase to show that unknown notification targets are ignored Winsley von Spee
e775d9190f02 Noissue - Enable source map for the tests Hubert Figuière
3af11978db00 Noissue - Updated copyright year Wladimir Palant
8a9c308bb8fb Noissue - Reorder chromium_process.js imports Dave Barker
b935a0402215 Noissue - Throw an error if the client code doesn't load Hubert Figuière
1a633dad14b4 Noissue - Remove unused reportError Hubert Figuière

Hints for testers

  • Issues #5459, #5460, #5558 implement changes to notification handling. Testing the notification mechanism is non-trivial unfortunately.
  • Issues #5079, #5339, #5381, #5422, #5436, #5438, #5516 all affect element hiding emulation functionality, please refer to the descriptions in the respective issues.
  • Issue #5000 isn't part of the list but has been resolved implicitly by #5438 and #5339 and should be retested.
  • The commits without issue have no impact on the build or affect functionality already covered by one of the issues above.

Change History (20)

comment:1 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:2 Changed 2 years ago by hfiguiere

  • Cc fhd trev sebastian added

comment:3 Changed 2 years ago by hfiguiere

  • Cc kzar added

comment:4 Changed 2 years ago by hfiguiere

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:5 Changed 2 years ago by hfiguiere

  • Description modified (diff)

comment:6 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5584 - Update adblockpluscore dependency to hg:217eff0504a5

comment:7 Changed 2 years ago by hfiguiere

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:8 Changed 2 years ago by hfiguiere

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:9 Changed 2 years ago by hfiguiere

  • Owner set to hfiguiere

We should revert the patch for now. We are missing issue #5535

comment:10 Changed 2 years ago by hfiguiere

  • Blocked By 5535 added

comment:12 Changed 2 years ago by sebastian

  • Priority changed from Unknown to P2
  • Ready set

comment:13 Changed 2 years ago by trev

  • Description modified (diff)

comment:14 Changed 2 years ago by kzar

This isn't blocked by 5535, here's an example of how to do the dependency update before the webpack stuff landed.

diff --git a/dependencies b/dependencies
index b4b2343f..40467730 100644
--- a/dependencies
+++ b/dependencies
@@ -2,5 +2,5 @@ _root = hg:https://hg.adblockplus.org/ git:https://github.com/adblockplus/
 _self = buildtools/ensure_dependencies.py
 buildtools = buildtools hg:4276509ccff9 git:3eb4cc7
 adblockplus = adblockplus hg:c8ec667756d3 git:9a42447
-adblockpluscore = adblockpluscore hg:442aa536668a git:65f427d
+adblockpluscore = adblockpluscore hg:217eff0504a5 git:48eddbc
 adblockplusui = adblockplusui hg:9976daa84915 git:1ec5c0d
diff --git a/include.preload.js b/include.preload.js
index 6d318808..f0e93a94 100644
--- a/include.preload.js
+++ b/include.preload.js
@@ -15,7 +15,8 @@
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* globals ElemHideEmulation, splitSelector */
+let {splitSelector} = require("common");
+let {ElemHideEmulation} = require("content_elemHideEmulation");
 
 "use strict";
 
diff --git a/metadata.chrome b/metadata.chrome
index 4f9b64e2..6ed4feb1 100644
--- a/metadata.chrome
+++ b/metadata.chrome
@@ -115,7 +115,8 @@ lib/adblockplus.js[injectinfomodule] = true
 lib/adblockplus.js[autoload] = filterListener,synchronizer,requestBlocker,popupBlocker,subscriptionInit,filterComposer,stats,uninstall,csp,cssInjection
 include.preload.js = include.preload.js inject.preload.js
 include.postload.js = subscriptionLink.postload.js composer.postload.js
-elemHideEmulation.js = adblockpluscore/lib/common.js adblockpluscore/chrome/content/elemHideEmulation.js
+elemHideEmulation.js = adblockpluscore/lib/common.js adblockpluscore/lib/content/elemHideEmulation.js
+elemHideEmulation.js[module] = true
 
 [import_locales]
 adblockplus/chrome/locale/*/global.properties = subscription_invalid_location

(Only briefly tested, I would recommend testing further yourself.)

comment:15 Changed 2 years ago by kzar

  • Blocked By 5535 removed

comment:16 Changed 2 years ago by kzar

(Also take a look at #5089.)

comment:17 Changed 2 years ago by abpbot

A commit referencing this issue has landed:
Issue 5584 - Update adblockpluscore dependency to hg:217eff0504a

comment:18 Changed 2 years ago by hfiguiere

  • Resolution set to fixed
  • Status changed from reopened to closed

comment:19 Changed 2 years ago by Ross

The element hiding parts of this ticket seems fine in Chrome/Opera/FirefoxWebEx, still looking at notification parts.

comment:20 Changed 2 years ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Element hiding fixes/changes are working as expected (except in older browsers - #5773). The notification features added appear to work as expected. Related issues (like #5000) are fixed.

ABP 1.13.3.1838
Chrome 52 / 61 / Windows 7/10
Opera 40 / 47 / Windows 7/10

Note: See TracTickets for help on using tickets.