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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 2 months ago by Manish Jethani
Modified:
2 years, 1 month ago
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
2 years, 2 months ago (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 ...
2 years, 2 months ago (2017-08-01 08:24:21 UTC) #2
Manish Jethani
Patch Set 2: Add comment
2 years, 2 months ago (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 ...
2 years, 2 months ago (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 ...
2 years, 2 months ago (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 ...
2 years, 2 months ago (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 ...
2 years, 2 months ago (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 ...
2 years, 2 months ago (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
2 years, 2 months ago (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/
2 years, 1 month ago (2017-08-21 14:01:08 UTC) #10
Manish Jethani
2 years, 1 month ago (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.
Sign in to reply to this message.

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