Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#178 closed defect (fixed)

Deal with missing frame URL gracefully

Reported by: trev Assignee: sebastian
Priority: P1 Milestone: Adblock-Plus-1.8-for-Chrome-Opera-Safari
Module: Platform Keywords: chrome
Cc: smultron45@… Blocked By:
Blocking: Platform:
Ready: yes Confidential: no
Tester: Verified working: no
Review URL(s):

http://codereview.adblockplus.org/5133931611422720/

Description

Environment

Adblock Plus for Chrome 1.7.4

How to reproduce

See http://code.google.com/p/chromium/issues/detail?id=352080

Observed behaviour

isWhitelisted() calls stripFragmentFromURL() with a null frame URL which causes an exception. In future Chrome versions that exception will cause the request to be cancelled.

Expected behaviour

isWhitelisted() should ignore frames without a URL. It might also be a good idea to check why we have a null frame URL in this scenario and whether there is some issue with our frame tracking code.

Change History (9)

comment:1 Changed 6 years ago by trev

  • Keywords chrome added

comment:2 Changed 6 years ago by sebastian

  • Resolution set to invalid
  • Status changed from new to closed

This currently happens when encountering an unknown frame in an unknown tab. However considering that this is a Chrome-specific issue and that isWhitelisted() isn't the only case where unknown frames break our code, it should rather be handled into the abstraction layer.

This was already fixed properly with http://codereview.adblockplus.org/5133931611422720/, by retrieving existing frames at startup, and ignoring frames that are still unkown.

Version 3, edited 6 years ago by sebastian (previous) (next) (diff)

comment:3 Changed 6 years ago by trev

  • Resolution invalid deleted
  • Review URL(s) modified (diff)
  • Status changed from closed to reopened

Even if my proposed solution isn't optimal, that doesn't make the issue itself invalid ;)

It merely means that it will be resolved by this review. Reopening.

comment:4 Changed 6 years ago by trev

  • Owner set to sebastian
  • Status changed from reopened to assigned

comment:5 Changed 6 years ago by trev

  • Status changed from assigned to reviewing

comment:6 Changed 6 years ago by mapx

  • Cc smultron45@… added

comment:7 Changed 6 years ago by sebastian

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

comment:8 Changed 6 years ago by trev

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

comment:9 Changed 5 years ago by sebastian

  • Ready set
Note: See TracTickets for help on using tickets.