Issue 1181 - Change polyfill for IE8 (fix settings page)

Issue 1181 - Change polyfill for IE8 (fix settings page)

[Felix Dahlke, 2014-08-08 12:34:08]

http://codereview.adblockplus.org/6196733759455232/diff/5629499534213120/html...
File html/static/js/ieEventListenerPolyfill.js (left):

http://codereview.adblockplus.org/6196733759455232/diff/5629499534213120/html...
html/static/js/ieEventListenerPolyfill.js:1: if (typeof window.addEventListener
!= "function")
I'm not sure why this works in IE8 frankly, window shouldn't have anything from
Element.prototype, and we're explicitly invoking window.addEventListener.

But that's easy to fix, this should do:

if (Window)
{
  Window.prototype.addEventListener = Element.prototype.addEventListener;
  Window.prototype.removeEventListener = Element.prototype.removeEventListener;
}

From:
https://developer.mozilla.org/en/docs/Web/API/EventTarget.addEventListener#Ol...

[Oleksandr, 2014-08-08 13:11:05]

http://codereview.adblockplus.org/6196733759455232/diff/5629499534213120/html...
File html/static/js/ieEventListenerPolyfill.js (left):

http://codereview.adblockplus.org/6196733759455232/diff/5629499534213120/html...
html/static/js/ieEventListenerPolyfill.js:1: if (typeof window.addEventListener
!= "function")
It actually wasn't working on firstrun page, my bad. I have modified your
suggestion a bit, wouldn't be good to replace addEventListener if it was already
implemented (IE9+).

Also, since there was 2 "load" even handlers, where one was expecting another
one to run first, I have combined them into one.
On 2014/08/08 12:34:08, Felix H. Dahlke wrote:
> I'm not sure why this works in IE8 frankly, window shouldn't have anything
from
> Element.prototype, and we're explicitly invoking window.addEventListener.
> 
> But that's easy to fix, this should do:
> 
> if (Window)
> {
>   Window.prototype.addEventListener = Element.prototype.addEventListener;
>   Window.prototype.removeEventListener =
Element.prototype.removeEventListener;
> }
> 
> From:
>
https://developer.mozilla.org/en/docs/Web/API/EventTarget.addEventListener#Ol...

[Felix Dahlke, 2014-08-08 13:22:32]

http://codereview.adblockplus.org/6196733759455232/diff/5649050225344512/html...
File html/static/js/ieEventListenerPolyfill.js (right):

http://codereview.adblockplus.org/6196733759455232/diff/5649050225344512/html...
html/static/js/ieEventListenerPolyfill.js:17: if (Window.addEventListener !=
"function")
Either Window.prototype or window, not Window.

Anyway - we can probably leave this as it was, eh? Doesn't seem to be related to
the IE8 fix, and if we don't use addEventListener on anything but window, what
we had before was fine.

[Felix Dahlke, 2014-08-08 13:58:35]

http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html...
File html/static/js/ieEventListenerPolyfill.js (right):

http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html...
html/static/js/ieEventListenerPolyfill.js:17: if (typeof
Element.prototype.addEventListener != "function")
My question remains: Is this still necessary to fix things in IE8? We use
window.addEventListener everywhere explicitly from what I can tell.

[Oleksandr, 2014-08-08 14:05:17]

http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html...
File html/static/js/ieEventListenerPolyfill.js (right):

http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html...
html/static/js/ieEventListenerPolyfill.js:17: if (typeof
Element.prototype.addEventListener != "function")
Oh, I miss understood you before then. Yes, it is necessary. We are not using
window.addEventListener everywhere, we also use addEventListener on elements.
Like an event listener for subscriptions combo box, or a AA checkbox or a button
to manage domains (IESettings.js).


On 2014/08/08 13:58:35, Felix H. Dahlke wrote:
> My question remains: Is this still necessary to fix things in IE8? We use
> window.addEventListener everywhere explicitly from what I can tell.

[Felix Dahlke, 2014-08-08 14:07:01]

http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html...
File html/static/js/ieEventListenerPolyfill.js (right):

http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html...
html/static/js/ieEventListenerPolyfill.js:17: if (typeof
Element.prototype.addEventListener != "function")
On 2014/08/08 14:05:17, Oleksandr wrote:
> Oh, I miss understood you before then. Yes, it is necessary. We are not using
> window.addEventListener everywhere, we also use addEventListener on elements.
> Like an event listener for subscriptions combo box, or a AA checkbox or a
button
> to manage domains (IESettings.js).
> 
> 
> On 2014/08/08 13:58:35, Felix H. Dahlke wrote:
> > My question remains: Is this still necessary to fix things in IE8? We use
> > window.addEventListener everywhere explicitly from what I can tell.
> 

Ah, I see! Misunderstanding then.

http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html...
html/static/js/ieEventListenerPolyfill.js:19: Element.prototype.addEventListener
= function(type, handler, capture)
Can we assign this to window.addEventListener, and removeEventListener below
respectively?

[Oleksandr, 2014-08-08 14:19:34]

http://codereview.adblockplus.org/6196733759455232/diff/5733935958982656/html...
File html/static/js/ieEventListenerPolyfill.js (right):

http://codereview.adblockplus.org/6196733759455232/diff/5733935958982656/html...
html/static/js/ieEventListenerPolyfill.js:19: Element.prototype.addEventListener
= window.addEventListener;
Looks like we've tried every combination now :)

[Felix Dahlke, 2014-08-08 14:21:12]

LGTM

http://codereview.adblockplus.org/6196733759455232/diff/5733935958982656/html...
File html/static/js/ieEventListenerPolyfill.js (right):

http://codereview.adblockplus.org/6196733759455232/diff/5733935958982656/html...
html/static/js/ieEventListenerPolyfill.js:19: Element.prototype.addEventListener
= window.addEventListener;
On 2014/08/08 14:19:34, Oleksandr wrote:
> Looks like we've tried every combination now :)

Yes, I like this one best :)