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

Issue 5545110112567296: Issue #1204 - Remove A/B testing for donation button in firstrun page in IE (Closed)

Created:
Oct. 27, 2014, 1:16 a.m. by Oleksandr
Modified:
Nov. 4, 2014, 11:37 a.m.
Reviewers:
sergei, Thomas Greiner
CC:
Felix Dahlke
Visibility:
Public.

Description

Issue #1204 - Remove A/B testing for donation button in firstrun page in IE

Patch Set 1 #

Total comments: 10

Patch Set 2 : Cleanup. Addressing comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -46 lines) Patch
M html/static/css/firstRun.css View 1 4 chunks +8 lines, -16 lines 0 comments Download
M html/static/js/firstRun.js View 1 4 chunks +7 lines, -14 lines 0 comments Download
M html/templates/firstRun.html View 1 1 chunk +7 lines, -16 lines 0 comments Download

Messages

Total messages: 6
Oleksandr
Oct. 27, 2014, 1:18 a.m. (2014-10-27 01:18:01 UTC) #1
sergei
LGTM Interesting that IE shows me a dialog "Internet Explorer restricted this webpage from running ...
Oct. 27, 2014, 11:24 p.m. (2014-10-27 23:24:22 UTC) #2
Oleksandr
Adding Thomas as a reviewer
Nov. 1, 2014, 9:14 a.m. (2014-11-01 09:14:35 UTC) #3
Thomas Greiner
http://codereview.adblockplus.org/5545110112567296/diff/5629499534213120/html/static/css/firstRun.css File html/static/css/firstRun.css (right): http://codereview.adblockplus.org/5545110112567296/diff/5629499534213120/html/static/css/firstRun.css#newcode64 html/static/css/firstRun.css:64: #share, Nit: Remove the trailing comma. http://codereview.adblockplus.org/5545110112567296/diff/5629499534213120/html/static/css/firstRun.css#newcode71 html/static/css/firstRun.css:71: html.share ...
Nov. 3, 2014, 5:14 p.m. (2014-11-03 17:14:11 UTC) #4
Oleksandr
http://codereview.adblockplus.org/5545110112567296/diff/5629499534213120/html/static/css/firstRun.css File html/static/css/firstRun.css (right): http://codereview.adblockplus.org/5545110112567296/diff/5629499534213120/html/static/css/firstRun.css#newcode109 html/static/css/firstRun.css:109: margin: 35px 0px; On 2014/11/03 17:14:12, Thomas Greiner wrote: ...
Nov. 4, 2014, 11:18 a.m. (2014-11-04 11:18:24 UTC) #5
Thomas Greiner
Nov. 4, 2014, 11:27 a.m. (2014-11-04 11:27:46 UTC) #6
LGTM

http://codereview.adblockplus.org/5545110112567296/diff/5629499534213120/html...
File html/static/js/firstRun.js (right):

http://codereview.adblockplus.org/5545110112567296/diff/5629499534213120/html...
html/static/js/firstRun.js:111: "share-connection": "first-run-share2-or",
On 2014/11/04 11:18:24, Oleksandr wrote:
> On 2014/11/03 17:14:12, Thomas Greiner wrote:
> > Did you check whether IE8 can deal with trailing commas in object literals?
> 
> It worked fine in IE8. Removed it anyway.

Great, thanks.

Powered by Google App Engine
This is Rietveld