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

Issue 29333043: Issue 3452 - Bring back $ping type option (Core changes) (Closed)

Created:
Dec. 23, 2015, 1:04 p.m. by Wladimir Palant
Modified:
Dec. 23, 2015, 2:18 p.m.
Reviewers:
Sebastian Noack
Visibility:
Public.

Description

Issue 3452 - Bring back $ping type option (Core changes)

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -4 lines) Patch
M chrome/locale/en-US/global.properties View 1 chunk +1 line, -0 lines 0 comments Download
M lib/contentPolicy.js View 2 chunks +8 lines, -3 lines 3 comments Download
M lib/filterClasses.js View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4
Wladimir Palant
Dec. 23, 2015, 1:04 p.m. (2015-12-23 13:04:50 UTC) #1
Sebastian Noack
LGTM, with or without the comment addressed. https://codereview.adblockplus.org/29333043/diff/29333044/lib/contentPolicy.js File lib/contentPolicy.js (right): https://codereview.adblockplus.org/29333043/diff/29333044/lib/contentPolicy.js#newcode129 lib/contentPolicy.js:129: // same ...
Dec. 23, 2015, 1:34 p.m. (2015-12-23 13:34:45 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29333043/diff/29333044/lib/contentPolicy.js File lib/contentPolicy.js (right): https://codereview.adblockplus.org/29333043/diff/29333044/lib/contentPolicy.js#newcode129 lib/contentPolicy.js:129: // same concept - merely generalized. On 2015/12/23 13:34:45, ...
Dec. 23, 2015, 2:14 p.m. (2015-12-23 14:14:34 UTC) #3
Sebastian Noack
Dec. 23, 2015, 2:18 p.m. (2015-12-23 14:18:19 UTC) #4
Message was sent while issue was closed.
https://codereview.adblockplus.org/29333043/diff/29333044/lib/contentPolicy.js
File lib/contentPolicy.js (right):

https://codereview.adblockplus.org/29333043/diff/29333044/lib/contentPolicy.j...
lib/contentPolicy.js:129: // same concept - merely generalized.
On 2015/12/23 14:14:34, Wladimir Palant wrote:
> On 2015/12/23 13:34:45, Sebastian Noack wrote:
> > I'm inclined to also add a note that we cannot distinguis BEACON and PING on
> > Chrome anyway.
> 
> That means fairly little for Firefox - we wouldn't dumb down our Firefox
> extension because of Chrome if distinguishing the types made sense.

Well, given that Chrome is our largest platform, I think the feasibility of
implementing a filter there, should be something we should consider, when adding
new filters.

Powered by Google App Engine
This is Rietveld