Opened on 11/17/2015 at 04:41:11 PM

Closed on 11/18/2015 at 09:59:41 AM

#3334 closed defect (fixed)

Several warnings in Adblock Plus for Chrome/Safari/Opera

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

https://codereview.adblockplus.org/29330321/

Description (last modified by kzar)

Background

During testing for the 1.9.4 release I have spotted several warnings / errors:

  • The test page trys to include lib/io.js but fails.
  • lib/whitelisting.js tries to use the getKey function but it is not defined locally, only exported.
  • webrequest.js does not appear to require all the modules it requires. I saw warnings that isFrameWhitelisted, getKey, defaultMatcher, stringifyURL, isThirdParty and extractHostFromFrame are undefined~~
  • The first run page trys to load skin/abp-128.png but it doesn't exist.

It would be nice to solve some of these before the release.

Attachments (0)

Change History (4)

comment:1 Changed on 11/17/2015 at 04:45:39 PM by kzar

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

comment:2 in reply to: ↑ description Changed on 11/17/2015 at 06:54:46 PM by trev

Replying to kzar:

  • webrequest.js does not appear to require all the modules it requires.

Not sure what produced these warnings, but webrequest.js is running in a shared context (background page) along with a number of other scripts. The modules are required in background.js.

  • The test page trys to include lib/io.js but fails.

That should be because the module in question is compiled into lib/adblockplus.js.

  • lib/whitelisting.js tries to use the getKey function but it is not defined locally, only exported.

It's defined in background.js but - sure, better to have it defined locally.

These need to be resolved before the release.

Given that none of these appear to be new or particularly critical from what I can tell, I cannot really agree on the urgency. Still, would be nice to have them resolved of course.

comment:3 Changed on 11/17/2015 at 09:13:51 PM by kzar

  • Description modified (diff)

comment:4 Changed on 11/18/2015 at 09:59:41 AM by kzar

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

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from kzar.
 
Note: See TracTickets for help on using tickets.