Opened on 11/15/2016 at 03:16:42 PM
Closed on 02/28/2017 at 12:52:38 PM
Last modified on 03/13/2017 at 08:33:11 AM
#4647 closed defect (fixed)
onBeforeRequest listener throwing exception for frames without a URL
| Reported by: | kzar | Assignee: | kzar |
|---|---|---|---|
| Priority: | P1 | Milestone: | Adblock-Plus-1.13-for-Chrome-Opera |
| Module: | Platform | Keywords: | |
| Cc: | trev, sebastian, BrentM, rhana@getadblock.com, arthur | Blocked By: | |
| Blocking: | #4598 | Platform: | Chrome |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): | |||
Description
Environment
adblockpluschrome 12a8449bde06
Debian Stretch
Chrome Version 54.0.2840.59 (64-bit)
How to reproduce
- Open the console for the extension's background page.
- Browse the web for a while.
Observed behaviour
Exceptions being thrown by the onBeforeRequest handler, for frames without a URL property.
Expected behaviour
Those exceptions should not be thrown.
Notes
The onBeforeRequest listener assumes that the originating frame will always have a URL. This is no longer true however since we now create frames with only the parent property in the onBeforeNavigate handler. Therefore I think this is a regression caused by Issue 4386 - Fixed determining document domain, particularly after being redirected.
We should now either ignore requests from frames without a URL or adapt the onBeforeRequest handler so that it does not assume the frame URL will be available. (Might be worthwhile since the parent frame might already have a URL.)
Attachments (0)
Change History (8)
comment:1 Changed on 11/15/2016 at 03:53:35 PM by kzar
- Blocking 4598 added
comment:2 Changed on 11/15/2016 at 04:15:02 PM by kzar
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:4 Changed on 11/17/2016 at 04:06:11 PM by kzar
- Cc rhana@getadblock.com added
comment:5 Changed on 11/21/2016 at 10:24:57 AM by arthur
- Cc arthur added
comment:6 Changed on 01/07/2017 at 10:35:27 AM by sebastian
- Milestone set to Adblock-Plus-for-Chrome-Opera-next
comment:7 Changed on 02/28/2017 at 12:52:38 PM by kzar
- Resolution set to fixed
- Status changed from reviewing to closed
comment:8 Changed on 03/13/2017 at 08:31:57 AM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Looks fixed. Could not reproduce exception.
ABP 1.12.4.1739
Chrome 49 / 56 / Windows 10
Chrome 56 / OS X 10.12
Chrome 56 / Ubuntu 16.04
Opera 37 / 41 / Windows 7
Safari 10 / OS X 10.12
A commit referencing this issue has landed:
Issue 4598,4599,4647,4804 - Work around onCommitted flaw