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

Issue 29739594: Issue 6543 - Match requests without tabId/frameId in their originating context (Closed)

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

Description

Issue 6543 - Match requests without tabId/frameId in their originating context

Patch Set 1 #

Total comments: 10

Patch Set 2 : Use URL patterns to find tabs for initiator #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+123 lines, -81 lines) Patch
M lib/csp.js View 1 chunk +2 lines, -1 line 0 comments Download
M lib/cssInjection.js View 1 chunk +2 lines, -2 lines 0 comments Download
M lib/filterComposer.js View 2 chunks +2 lines, -2 lines 0 comments Download
M lib/popupBlocker.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/requestBlocker.js View 1 3 chunks +69 lines, -44 lines 4 comments Download
M lib/url.js View 2 chunks +4 lines, -3 lines 0 comments Download
M lib/whitelisting.js View 3 chunks +43 lines, -28 lines 0 comments Download

Messages

Total messages: 7
Sebastian Noack
1 year, 9 months ago (2018-04-02 02:56:10 UTC) #1
kzar
https://codereview.adblockplus.org/29739594/diff/29739595/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29739594/diff/29739595/lib/requestBlocker.js#newcode87 lib/requestBlocker.js:87: if (details.originUrl) Nit: Please use braces since the body ...
1 year, 9 months ago (2018-04-03 12:49:30 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29739594/diff/29739595/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29739594/diff/29739595/lib/requestBlocker.js#newcode87 lib/requestBlocker.js:87: if (details.originUrl) On 2018/04/03 12:49:30, kzar wrote: > Nit: ...
1 year, 9 months ago (2018-04-04 01:04:45 UTC) #3
kzar
Otherwise LGTM https://codereview.adblockplus.org/29739594/diff/29739595/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29739594/diff/29739595/lib/requestBlocker.js#newcode96 lib/requestBlocker.js:96: return browser.tabs.query({}).then(tabs => On 2018/04/04 01:04:45, Sebastian ...
1 year, 9 months ago (2018-04-05 10:31:51 UTC) #4
Sebastian Noack
https://codereview.adblockplus.org/29739594/diff/29741715/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29739594/diff/29741715/lib/requestBlocker.js#newcode94 lib/requestBlocker.js:94: return Promise.resolve([]); On 2018/04/05 10:31:51, kzar wrote: > I ...
1 year, 9 months ago (2018-04-05 16:56:33 UTC) #5
kzar
https://codereview.adblockplus.org/29739594/diff/29741715/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29739594/diff/29741715/lib/requestBlocker.js#newcode94 lib/requestBlocker.js:94: return Promise.resolve([]); On 2018/04/05 16:56:33, Sebastian Noack wrote: > ...
1 year, 9 months ago (2018-04-09 14:39:19 UTC) #6
Sebastian Noack
1 year, 9 months ago (2018-04-09 21:08:20 UTC) #7
Message was sent while issue was closed.
https://codereview.adblockplus.org/29739594/diff/29741715/lib/requestBlocker.js
File lib/requestBlocker.js (right):

https://codereview.adblockplus.org/29739594/diff/29741715/lib/requestBlocker....
lib/requestBlocker.js:94: return Promise.resolve([]);
On 2018/04/09 14:39:18, kzar wrote:
> On 2018/04/05 16:56:33, Sebastian Noack wrote:
> > ...IMO it's not worth to bother about them if it's just for outdated Chrome
> versions.
> 
> Sounds reasonable to me, maybe add a note to the issue to explain that know
> limitation?

There you go: https://issues.adblockplus.org/ticket/6544?action=diff&version=3
Sign in to reply to this message.

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