Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(301)

Issue 29750566: Issue 6565 - Ensure the URL is known for requests we might block (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 7 months ago by kzar
Modified:
1 year, 7 months ago
Reviewers:
Sebastian Noack
Visibility:
Public.

Description

Issue 6565 - Ensure the URL is known for requests we might block

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M lib/requestBlocker.js View 1 chunk +1 line, -1 line 5 comments Download

Messages

Total messages: 5
kzar
Patch Set 1
1 year, 7 months ago (2018-04-12 11:51:52 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29750566/diff/29750567/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29750566/diff/29750567/lib/requestBlocker.js#newcode154 lib/requestBlocker.js:154: page = new ext.Page({id: details.tabId, url: details.url}); Shouldn't this ...
1 year, 7 months ago (2018-04-12 13:27:09 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29750566/diff/29750567/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29750566/diff/29750567/lib/requestBlocker.js#newcode154 lib/requestBlocker.js:154: page = new ext.Page({id: details.tabId, url: details.url}); On 2018/04/12 ...
1 year, 7 months ago (2018-04-12 13:38:09 UTC) #3
kzar
https://codereview.adblockplus.org/29750566/diff/29750567/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29750566/diff/29750567/lib/requestBlocker.js#newcode154 lib/requestBlocker.js:154: page = new ext.Page({id: details.tabId, url: details.url}); On 2018/04/12 ...
1 year, 7 months ago (2018-04-13 15:36:38 UTC) #4
Sebastian Noack
1 year, 7 months ago (2018-04-14 03:21:26 UTC) #5
Message was sent while issue was closed.
https://codereview.adblockplus.org/29750566/diff/29750567/lib/requestBlocker.js
File lib/requestBlocker.js (right):

https://codereview.adblockplus.org/29750566/diff/29750567/lib/requestBlocker....
lib/requestBlocker.js:154: page = new ext.Page({id: details.tabId, url:
details.url});
On 2018/04/13 15:36:37, kzar wrote:
> On 2018/04/12 13:27:09, Sebastian Noack wrote:
> > Shouldn't this be rather the originUrl than the url of this request? But
then
> > again I don't get where the error is comming from, since we fall back to the
> > originURL anyway, and the code path that touches the page object in
> > checkWhitelisted() isn't even hit as long as an originUrl is given.
> 
> Yea, good point. Well since we now know why you couldn't reproduce this to
> initially (disabled new tab page) perhaps you could try again to figure out
> what's happening? I'd rather not change something that I don't fully
understand
> in case I'm missing something important.

How do I get that new tab page?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5