Opened 4 years ago

Closed 4 years ago

#3998 closed change (fixed)

cssProperties.js content script shouldn't assume that messaging will automatically know the context

Reported by: trev Assignee: trev
Priority: P3 Milestone:
Module: Core Keywords:
Cc: sebastian Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29340895/

Description (last modified by trev)

Background

As implemented right now, cssProperties.js content script will call ext.backgroundPage.sendMessage() in order to retrieve patterns to be applied. Information about the current window isn't being sent, the assumption is that the background page will know "automatically". This works well in Chrome where there is an instance of the content script in each window, so the context can be deduced from the sender. In Firefox we only run one instance of the content script per process however and have to resort to hacks in order to get the context.

What to change

Rather than use ext.backgroundPage.sendMessage() directly, have the CSSPropertyFilters accept a getFiltersFunc parameter. This should be a function that will retrieve the filters to be applied on the current page. On Chrome this will be a minimal wrapper around ext.backgroundPage.sendMessage(), on Firefox a function bound to the current window.

Integration notes

CSSPropertyFilters constructor has to be passed a new parameter, before addSelectorsFunc. It should be something like:

callback =>
{
  ext.backgroundPage.sendMessage(
  {
    type: "filters.get",
    what: "cssproperties"
  }, callback);
}

Change History (3)

comment:1 Changed 4 years ago by trev

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

comment:2 Changed 4 years ago by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/adblockpluscore/rev/d576e2dac412

comment:3 Changed 4 years ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.