Opened 3 years ago

Closed 2 years ago

Last modified 23 months ago

#2397 closed change (fixed)

Integrate CSS property rule handling in Chrome/Opera/Safari

Reported by: trev Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-1.10-for-Chrome-Opera-Safari
Module: Platform Keywords:
Cc: sebastian, greiner Blocked By: #2395, #2396
Blocking: #2388, #3596 Platform: Unknown
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29331619/
https://codereview.adblockplus.org/29332572/

Description (last modified by kzar)

Background

We are working on adding support for CSS property filters #2388, which will allow elements to be hidden when matching CSS style rules are applied to them.

Adding support for this has required multiple changes, mostly to the adblockplus and adblockplusui repositories. We now want to bring these changes into our Adblock Plus for Chrome by updating those dependencies and integrating the code.

What to change

  • Update the adblockplus dependency to include the changes in #2392, #2393 and #2395. Specifically to revision c0345d415b32 (and in Git 245e0d7). This will include several other commits, but none with relevant changes.
  • Update the adblockplustests dependency to include the changes in #2394. Specifically to revision 2fa49573d6c1 (and in Git 8343d6e). This will include several other commits but none that are relevant to the Chrome extension. (There are a few changes to Firefox related files and metadata for es10 support and an added README.)
  • Integrate the code into include.preload.js, starting from this diff https://pastebin.mozilla.org/8850328
  • Make the required changes to metadata.common to include cssProperties.js and cssRules.js

What to test

  • That the extension continues to function properly on Chrome, Opera and Safari.
  • That all the tests pass, including the new CSS property filter ones.
  • That property filters work on Dave's test page for Chrome, Opera and Safari. (Test both with the block element tool, refreshing the page and adding the rules manually.)

Change History (15)

comment:1 Changed 2 years ago by kzar

  • Owner set to kzar
  • Ready unset
  • Tester set to Unknown

(This issue's description isn't ready as it stands. I'm going to try and update it after reading the surrounding issues, but any help would with that would likely save time and be appreciated.)

comment:2 Changed 2 years ago by kzar

  • Cc greiner added
  • Description modified (diff)
  • Ready set

With help from Thomas we made some progress with this today, I successfully hid a element using a simple CSS property filter on my test page.

I've found a few small changes to be made to the code in reviews for #2392, #2393 and #2395 but it seems to be more or less working. I'm now looking to see how I can test things more thoroughly.

If you'd like to reproduce my environment to work on your issues or test things you can check out my branch https://github.com/kzar/adblockpluschrome/tree/2388-css-filters in GitHub. (I've updated the dependencies file, so building should use the 2388-css-filters branch for adblockplus and adblockplusui as well.)

comment:3 Changed 2 years ago by kzar

  • Description modified (diff)

comment:4 Changed 2 years ago by kzar

  • Description modified (diff)

comment:5 Changed 2 years ago by kzar

  • Description modified (diff)

comment:6 Changed 2 years ago by greiner

  • Description modified (diff)

Moved the testing-related part of the ticket description to the What to test section and linked to the dependecy update notes in #2381 instead of duplicating them here.

comment:7 Changed 2 years ago by kzar

  • Description modified (diff)

comment:8 Changed 2 years ago by kzar

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

comment:9 Changed 2 years ago by kzar

  • Description modified (diff)

comment:10 Changed 2 years ago by kzar

  • Review URL(s) modified (diff)

comment:12 Changed 2 years ago by kzar

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

comment:13 Changed 2 years ago by kzar

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next

comment:14 Changed 23 months ago by sebastian

  • Blocked By 3596 added

comment:15 Changed 23 months ago by sebastian

  • Blocked By 3596 removed
  • Blocking 3596 added
Note: See TracTickets for help on using tickets.