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

Issue 4859491858251776: Fix the approach used in ieFirstRun.js (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 7 months ago by Oleksandr
Modified:
5 years, 3 months ago
Reviewers:
Eric, Felix Dahlke
Visibility:
Public.

Description

Fix the approach used in ieFirstRun.js

Patch Set 1 #

Total comments: 3

Patch Set 2 : Use polyfill for addEventListener #

Total comments: 5

Patch Set 3 : Formatting fixed #

Total comments: 4

Patch Set 4 : Formatting nit addressed #

Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -68 lines) Patch
M html/static/js/firstRun.js View 1 3 chunks +1 line, -25 lines 0 comments Download
A html/static/js/ieEventListenerPolyfill.js View 1 1 chunk +15 lines, -0 lines 0 comments Download
M html/static/js/ieFirstRun.js View 1 2 3 1 chunk +37 lines, -43 lines 0 comments Download
M html/templates/firstRun.html View 1 1 chunk +1 line, -0 lines 0 comments Download
M html/templates/index.html View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 10
Oleksandr
This is based on https://issues.adblockplus.org/ticket/65
5 years, 6 months ago (2014-03-31 08:34:17 UTC) #1
Wladimir Palant
http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html/static/js/ieFirstRun.js File html/static/js/ieFirstRun.js (right): http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html/static/js/ieFirstRun.js#newcode25 html/static/js/ieFirstRun.js:25: window.addEventListener("load", function() This won't work in older IE versions. ...
5 years, 6 months ago (2014-03-31 09:50:29 UTC) #2
Felix Dahlke
On 2014/03/31 09:50:29, Wladimir Palant wrote: > http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html/static/js/ieFirstRun.js > File html/static/js/ieFirstRun.js (right): > > http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html/static/js/ieFirstRun.js#newcode25 ...
5 years, 3 months ago (2014-06-24 15:18:28 UTC) #3
Eric
http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html/static/js/ieFirstRun.js File html/static/js/ieFirstRun.js (right): http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html/static/js/ieFirstRun.js#newcode25 html/static/js/ieFirstRun.js:25: window.addEventListener("load", function() On 2014/03/31 09:50:29, Wladimir Palant wrote: > ...
5 years, 3 months ago (2014-06-25 16:21:06 UTC) #4
Oleksandr
5 years, 3 months ago (2014-07-02 08:41:06 UTC) #5
Felix Dahlke
http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html/static/js/firstRun.js File html/static/js/firstRun.js (right): http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html/static/js/firstRun.js#newcode37 html/static/js/firstRun.js:37: function removeListener(object, type, listener) There are multiple uses of ...
5 years, 3 months ago (2014-07-04 13:40:31 UTC) #6
Oleksandr
5 years, 3 months ago (2014-07-15 20:30:16 UTC) #7
Felix Dahlke
Almost there :) http://codereview.adblockplus.org/4859491858251776/diff/5718998062727168/html/static/js/ieFirstRun.js File html/static/js/ieFirstRun.js (right): http://codereview.adblockplus.org/4859491858251776/diff/5718998062727168/html/static/js/ieFirstRun.js#newcode20 html/static/js/ieFirstRun.js:20: } Indentation is still off here ...
5 years, 3 months ago (2014-07-16 11:05:17 UTC) #8
Oleksandr
http://codereview.adblockplus.org/4859491858251776/diff/5718998062727168/html/static/js/ieFirstRun.js File html/static/js/ieFirstRun.js (right): http://codereview.adblockplus.org/4859491858251776/diff/5718998062727168/html/static/js/ieFirstRun.js#newcode35 html/static/js/ieFirstRun.js:35: result.getMessage = function(section, param) This doesn't work. Settings.GetMessage is ...
5 years, 3 months ago (2014-07-16 11:19:41 UTC) #9
Felix Dahlke
5 years, 3 months ago (2014-07-18 12:43:36 UTC) #10
LGTM at last! :)

http://codereview.adblockplus.org/4859491858251776/diff/5718998062727168/html...
File html/static/js/ieFirstRun.js (right):

http://codereview.adblockplus.org/4859491858251776/diff/5718998062727168/html...
html/static/js/ieFirstRun.js:35: result.getMessage = function(section, param)
On 2014/07/16 11:19:41, Oleksandr wrote:
> This doesn't work. Settings.GetMessage is a method in our injected object.
> 
> On 2014/07/16 11:05:17, Felix H. Dahlke wrote:
> > Simpler:
> > 
> > result.getMessage = Settings.GetMessage;
> 

Oh :P
Sign in to reply to this message.

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