Opened on 03/20/2014 at 06:49:43 AM
Closed on 04/12/2014 at 08:49:10 AM
Last modified on 05/16/2014 at 01:44:13 PM
#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@gmail.com | Blocked By: | |
| Blocking: | Platform: | ||
| Ready: | yes | Confidential: | no |
| Tester: | Verified working: | no | |
| Review URL(s): | |||
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.
Attachments (0)
Change History (9)
comment:1 Changed on 03/20/2014 at 06:58:50 AM by trev
- Keywords chrome added
comment:2 Changed on 03/20/2014 at 03:05:13 PM by sebastian
- Resolution set to invalid
- Status changed from new to closed
Last edited on 03/20/2014 at 03:24:59 PM
by sebastian
comment:3 Changed on 03/20/2014 at 05:28:55 PM 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 on 03/20/2014 at 05:29:18 PM by trev
- Owner set to sebastian
- Status changed from reopened to assigned
comment:5 Changed on 03/20/2014 at 05:29:27 PM by trev
- Status changed from assigned to reviewing
comment:6 Changed on 04/02/2014 at 08:23:04 AM by mapx
- Cc smultron45@gmail.com added
comment:7 Changed on 04/12/2014 at 08:49:10 AM by sebastian
- Resolution set to fixed
- Status changed from reviewing to closed
comment:8 Changed on 04/13/2014 at 06:30:19 PM by trev
- Milestone set to Adblock-Plus-for-Chrome-Opera-Safari-next
comment:9 Changed on 05/16/2014 at 01:44:13 PM by sebastian
- Ready set
Note: See
TracTickets for help on using
tickets.
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 on startup, and ignoring frames that are still unknown.