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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years ago by Oleksandr
Modified:
5 years ago
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
5 years ago (2014-10-27 01:18:01 UTC) #1
sergei
LGTM Interesting that IE shows me a dialog "Internet Explorer restricted this webpage from running ...
5 years ago (2014-10-27 23:24:22 UTC) #2
Oleksandr
Adding Thomas as a reviewer
5 years ago (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 ...
5 years ago (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: ...
5 years ago (2014-11-04 11:18:24 UTC) #5
Thomas Greiner
5 years ago (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.
Sign in to reply to this message.

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