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

Issue 29342971: Fixes 4065 - Subscription links on Chrome 30 (Closed)

Created:
May 24, 2016, 11:48 a.m. by kzar
Modified:
June 1, 2016, 4:14 p.m.
Reviewers:
Sebastian Noack
Visibility:
Public.

Description

Fixes 4065 - Subscription links on Chrome 30

Patch Set 1 #

Patch Set 2 : Addressed feedback #

Total comments: 2

Patch Set 3 : Removed redundant comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -4 lines) Patch
M subscriptionLink.postload.js View 1 2 2 chunks +15 lines, -4 lines 0 comments Download

Messages

Total messages: 6
kzar
Patch Set 1
May 24, 2016, 11:48 a.m. (2016-05-24 11:48:58 UTC) #1
Sebastian Noack
Let me guess, that's only for links that have a non-standard protocol? Mind using the ...
May 24, 2016, 12:09 p.m. (2016-05-24 12:09:43 UTC) #2
kzar
Patch Set 2 : Addressed feedback
May 24, 2016, 12:45 p.m. (2016-05-24 12:45:55 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29342971/diff/29342987/subscriptionLink.postload.js File subscriptionLink.postload.js (right): https://codereview.adblockplus.org/29342971/diff/29342987/subscriptionLink.postload.js#newcode69 subscriptionLink.postload.js:69: // Decode URL parameters (Note: Old versions of Chrome ...
May 24, 2016, 12:48 p.m. (2016-05-24 12:48:06 UTC) #4
kzar
Patch Set 3 : Removed redundant comment https://codereview.adblockplus.org/29342971/diff/29342987/subscriptionLink.postload.js File subscriptionLink.postload.js (right): https://codereview.adblockplus.org/29342971/diff/29342987/subscriptionLink.postload.js#newcode69 subscriptionLink.postload.js:69: // Decode ...
May 24, 2016, 12:59 p.m. (2016-05-24 12:59:40 UTC) #5
Sebastian Noack
May 24, 2016, 1:03 p.m. (2016-05-24 13:03:07 UTC) #6
LGTM.

However, given that this is an old bug nobody except our own QA seem to have
noticed, let's push the fix after the release.

Powered by Google App Engine
This is Rietveld