Fix the approach used in ieFirstRun.js

Fix the approach used in ieFirstRun.js

[Oleksandr, 2014-03-31 08:34:17]

This is based on https://issues.adblockplus.org/ticket/65

[Wladimir Palant, 2014-03-31 09:50:29]

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

http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html...
html/static/js/ieFirstRun.js:25: window.addEventListener("load", function()
This won't work in older IE versions. How about you move the addListener()
function from firstRun.js here? It is best to change it into a polyfill so that
we can simply use the standard window.addEventListener() everywhere:

    if (typeof window.addEventListener != "function")
    {
      window.addEventListener = function(type, handler, capture)
      {
        this.attachEvent("on" + type, handler)
      };
    }

Same for removeListener() of course.

[Felix Dahlke, 2014-06-24 15:18:28]

On 2014/03/31 09:50:29, Wladimir Palant wrote:
>
http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html...
> File html/static/js/ieFirstRun.js (right):
> 
>
http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html...
> html/static/js/ieFirstRun.js:25: window.addEventListener("load", function()
> This won't work in older IE versions. How about you move the addListener()
> function from firstRun.js here? It is best to change it into a polyfill so
that
> we can simply use the standard window.addEventListener() everywhere:
> 
>     if (typeof window.addEventListener != "function")
>     {
>       window.addEventListener = function(type, handler, capture)
>       {
>         this.attachEvent("on" + type, handler)
>       };
>     }
> 
> Same for removeListener() of course.

Can you address Wladimir's comment Oleksandr? Then I'll look over things.

[Eric, 2014-06-25 16:21:06]

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

http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html...
html/static/js/ieFirstRun.js:25: window.addEventListener("load", function()
On 2014/03/31 09:50:29, Wladimir Palant wrote:
> This won't work in older IE versions.
And by older, that includes IE 8, unfortunately, and even (evidently)
compatibility view in IE 9.

See
http://stackoverflow.com/questions/6927637/addeventlistener-in-internet-explorer
for more detail.

[Felix Dahlke, 2014-07-04 13:40:31]

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

http://codereview.adblockplus.org/4859491858251776/diff/5629499534213120/html...
html/static/js/firstRun.js:37: function removeListener(object, type, listener)
There are multiple uses of this function below - shouldn't those use
window.removeEventListener() now?

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

http://codereview.adblockplus.org/4859491858251776/diff/5757334940811264/html...
html/static/js/ieFirstRun.js:4: prefs: {
Opening braces of multi-line object literals should go on their own line after
:. After = too actually, we only don't do it when it wouldn't work (e.g. after
"return").

http://codereview.adblockplus.org/4859491858251776/diff/5757334940811264/html...
html/static/js/ieFirstRun.js:5: Prefs: { documentation_link: "" }
No space after "{" or before "}" for object literals, by convention.

http://codereview.adblockplus.org/4859491858251776/diff/5757334940811264/html...
html/static/js/ieFirstRun.js:8: Utils: { appLocale: "" }
Indentation should be 2 spaces per level.

http://codereview.adblockplus.org/4859491858251776/diff/5757334940811264/html...
html/static/js/ieFirstRun.js:28: return Settings.GetMessage(section, param);
Indentation is off here.

http://codereview.adblockplus.org/4859491858251776/diff/5757334940811264/html...
html/static/js/ieFirstRun.js:29: }
Is that trailing whitespace?

[Felix Dahlke, 2014-07-16 11:05:17]

Almost there :)

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:20: }
Indentation is still off here :P

http://codereview.adblockplus.org/4859491858251776/diff/5718998062727168/html...
html/static/js/ieFirstRun.js:35: result.getMessage = function(section, param)
Simpler:

result.getMessage = Settings.GetMessage;

[Oleksandr, 2014-07-16 11:19:41]

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)
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;

[Felix Dahlke, 2014-07-18 12:43:36]

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