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

Issue 29421712: Issue 5184 - Support Firefox-specific webRequest types (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 4 weeks ago by Manish Jethani
Modified:
3 months ago
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Description

Issue 5184 - Support Firefox-specific webRequest types

Patch Set 1 #

Patch Set 2 : Move logic to requestBlocker.js #

Total comments: 8

Patch Set 3 : Address comments to Patch Set 2 #

Total comments: 4

Patch Set 4 : Report unmapped types as OTHER #

Total comments: 2

Patch Set 5 : Simplify logic a little bit #

Total comments: 4

Patch Set 6 : Use generator to populate map #

Total comments: 3

Patch Set 7 : Use for..in #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -7 lines) Patch
M ext/background.js View 1 1 chunk +1 line, -4 lines 0 comments Download
M lib/requestBlocker.js View 1 2 3 4 5 6 2 chunks +23 lines, -3 lines 0 comments Download

Messages

Total messages: 20
Manish Jethani
3 months, 4 weeks ago (2017-04-25 16:25:41 UTC) #1
Manish Jethani
Patch Set 1
3 months, 4 weeks ago (2017-04-25 16:28:10 UTC) #2
Sebastian Noack
I'd rather handle this mapping within RegExpFilter.typeMap (see lib/requestBlocker.js). This would simplify this change, but ...
3 months, 3 weeks ago (2017-04-29 21:49:09 UTC) #3
Manish Jethani
On 2017/04/29 21:49:09, Sebastian Noack wrote: > I'd rather handle this mapping within RegExpFilter.typeMap (see ...
3 months, 3 weeks ago (2017-05-01 23:25:18 UTC) #4
Sebastian Noack
On 2017/05/01 23:25:18, Manish Jethani wrote: > On 2017/04/29 21:49:09, Sebastian Noack wrote: > > ...
3 months, 1 week ago (2017-05-17 07:42:32 UTC) #5
Manish Jethani
Patch Set 2: Move logic to requestBlocker.js I've moved the entire mapping logic into requestBlocker.js ...
3 months ago (2017-05-18 01:16:23 UTC) #6
Sebastian Noack
https://codereview.adblockplus.org/29421712/diff/29440609/lib/requestBlocker.js File lib/requestBlocker.js (left): https://codereview.adblockplus.org/29421712/diff/29440609/lib/requestBlocker.js#oldcode33 lib/requestBlocker.js:33: RegExpFilter.typeMap.OBJECT_SUBREQUEST = RegExpFilter.typeMap.OBJECT; Unrelated, but note that this is ...
3 months ago (2017-05-18 12:44:24 UTC) #7
Manish Jethani
Patch Set 3: Address comments to Patch Set 2 Change variable name, map unmapped types ...
3 months ago (2017-05-18 20:52:04 UTC) #8
Sebastian Noack
https://codereview.adblockplus.org/29421712/diff/29441573/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29421712/diff/29441573/lib/requestBlocker.js#newcode32 lib/requestBlocker.js:32: // Chrome and Firefox can't distinguish between OBJECT_SUBREQUEST and ...
3 months ago (2017-05-18 21:33:42 UTC) #9
Manish Jethani
Patch Set 4: Report unmapped types as OTHER https://codereview.adblockplus.org/29421712/diff/29441573/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29421712/diff/29441573/lib/requestBlocker.js#newcode32 lib/requestBlocker.js:32: // ...
3 months ago (2017-05-18 21:49:34 UTC) #10
Sebastian Noack
https://codereview.adblockplus.org/29421712/diff/29440609/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29421712/diff/29440609/lib/requestBlocker.js#newcode39 lib/requestBlocker.js:39: ["web_manifest", "OTHER"], On 2017/05/18 20:52:04, Manish Jethani wrote: > ...
3 months ago (2017-05-19 11:30:46 UTC) #11
Manish Jethani
Patch Set 5: Simplify logic a little bit https://codereview.adblockplus.org/29421712/diff/29441576/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29421712/diff/29441576/lib/requestBlocker.js#newcode40 lib/requestBlocker.js:40: ]); ...
3 months ago (2017-05-19 16:54:42 UTC) #12
Manish Jethani
On 2017/05/19 11:30:46, Sebastian Noack wrote: > I think it is preferable to block request ...
3 months ago (2017-05-19 16:59:15 UTC) #13
Sebastian Noack
https://codereview.adblockplus.org/29421712/diff/29442629/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29421712/diff/29442629/lib/requestBlocker.js#newcode39 lib/requestBlocker.js:39: // Map unsupported resource types to types we support. ...
3 months ago (2017-05-19 17:47:41 UTC) #14
Manish Jethani
Patch Set 6: Use generator to populate map https://codereview.adblockplus.org/29421712/diff/29442629/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29421712/diff/29442629/lib/requestBlocker.js#newcode39 lib/requestBlocker.js:39: // ...
3 months ago (2017-05-19 23:44:15 UTC) #15
Sebastian Noack
https://codereview.adblockplus.org/29421712/diff/29442629/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29421712/diff/29442629/lib/requestBlocker.js#newcode39 lib/requestBlocker.js:39: // Map unsupported resource types to types we support. ...
3 months ago (2017-05-20 06:42:37 UTC) #16
Manish Jethani
Patch Set 7: Use for..in https://codereview.adblockplus.org/29421712/diff/29442629/lib/requestBlocker.js File lib/requestBlocker.js (right): https://codereview.adblockplus.org/29421712/diff/29442629/lib/requestBlocker.js#newcode39 lib/requestBlocker.js:39: // Map unsupported resource ...
3 months ago (2017-05-20 18:50:47 UTC) #17
Manish Jethani
On 2017/05/20 18:50:47, Manish Jethani wrote: > Patch Set 7: Use for..in Sorry, please ignore.
3 months ago (2017-05-20 18:51:37 UTC) #18
Manish Jethani
On 2017/05/20 18:51:37, Manish Jethani wrote: > On 2017/05/20 18:50:47, Manish Jethani wrote: > > ...
3 months ago (2017-05-20 18:55:41 UTC) #19
Sebastian Noack
3 months ago (2017-05-21 20:18:03 UTC) #20
LGTM
Sign in to reply to this message.

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