Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29502555: Issue 5464 - Ignore and whitelist WebRTC if not supported (Closed)

Created:
Aug. 1, 2017, 8:14 a.m. by Manish Jethani
Modified:
Aug. 21, 2017, 2:32 p.m.
Reviewers:
kzar
CC:
Sebastian Noack
Base URL:
https://hg.adblockplus.org/abp2blocklist
Visibility:
Public.

Description

Issue 5464 - Ignore and whitelist WebRTC if not supported If a filter covers all three of WebSocket, WebRTC, and at least one HTTP content type (e.g. a filter like just "foo"), we generate a rule with a generic URL scheme pattern, like "^[^:]+:(//)?". But since WebRTC is still not supported in the safari bookmark's version of core, we end up generating two rules in Safari for such a filter, one for "^wss?://" and one for "^https?://". If WebRTC is not supported, we need to ignore WebRTC. Also since we'll be generating a generic URL scheme pattern that covers WebRTC anyway, we need to whitelist WebRTC so no WebRTC URLs are inadvertently blocked.

Patch Set 1 #

Patch Set 2 : Add comment #

Patch Set 3 : Fix bug #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -3 lines) Patch
M lib/abp2blocklist.js View 1 2 4 chunks +25 lines, -3 lines 0 comments Download

Messages

Total messages: 11
Manish Jethani
Aug. 1, 2017, 8:14 a.m. (2017-08-01 08:14:25 UTC) #1
Manish Jethani
Patch Set 1 If a filter covers all three of WebSocket, WebRTC, and at least ...
Aug. 1, 2017, 8:24 a.m. (2017-08-01 08:24:21 UTC) #2
Manish Jethani
Patch Set 2: Add comment
Aug. 1, 2017, 8:40 a.m. (2017-08-01 08:40:53 UTC) #3
Manish Jethani
Patch Set 3: Fix bug Doh, there was a bug in the call to the ...
Aug. 1, 2017, 8:48 a.m. (2017-08-01 08:48:58 UTC) #4
kzar
Maybe I'm missing something, but if WebRTC connections aren't supported why do we need to ...
Aug. 1, 2017, 11:15 a.m. (2017-08-01 11:15:31 UTC) #5
Manish Jethani
On 2017/08/01 11:15:31, kzar wrote: > Maybe I'm missing something, but if WebRTC connections aren't ...
Aug. 1, 2017, 11:33 a.m. (2017-08-01 11:33:24 UTC) #6
kzar
After discussing with Sebastian I'm not sure about the best approach here. IMO the bitwise ...
Aug. 1, 2017, 1:08 p.m. (2017-08-01 13:08:54 UTC) #7
Manish Jethani
On 2017/08/01 13:08:54, kzar wrote: > Another option might be to modify typeMap in abp2blocklist.js ...
Aug. 1, 2017, 1:13 p.m. (2017-08-01 13:13:43 UTC) #8
Manish Jethani
On 2017/08/01 13:13:43, Manish Jethani wrote: > I'll open an issue for further discussion. https://issues.adblockplus.org/ticket/5464
Aug. 2, 2017, 6:32 a.m. (2017-08-02 06:32:32 UTC) #9
kzar
This can be closed, since we're going with the other approach[1] right? [1] - https://codereview.adblockplus.org/29503587/
Aug. 21, 2017, 2:01 p.m. (2017-08-21 14:01:08 UTC) #10
Manish Jethani
Aug. 21, 2017, 2:32 p.m. (2017-08-21 14:32:38 UTC) #11
Message was sent while issue was closed.
On 2017/08/21 14:01:08, kzar wrote:
> This can be closed, since we're going with the other approach[1] right? 
> 
> [1] - https://codereview.adblockplus.org/29503587/

True, I've closed it.

Powered by Google App Engine
This is Rietveld