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

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

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

Description

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

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fix the firstrun page as well #

Total comments: 1

Patch Set 3 : Add back addEventListener/removeEventListener handlers #

Total comments: 4

Patch Set 4 : Reuse code for addEventListener/removeEventListener #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -3 lines) Patch
M html/static/js/firstRun.js View 1 1 chunk +1 line, -3 lines 0 comments Download
M html/static/js/ieEventListenerPolyfill.js View 1 2 3 1 chunk +10 lines, -0 lines 2 comments Download
M html/static/js/ieFirstRun.js View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 10
Oleksandr
5 years, 6 months ago (2014-08-08 11:02:26 UTC) #1
Felix Dahlke
http://codereview.adblockplus.org/6196733759455232/diff/5629499534213120/html/static/js/ieEventListenerPolyfill.js File html/static/js/ieEventListenerPolyfill.js (left): http://codereview.adblockplus.org/6196733759455232/diff/5629499534213120/html/static/js/ieEventListenerPolyfill.js#oldcode1 html/static/js/ieEventListenerPolyfill.js:1: if (typeof window.addEventListener != "function") I'm not sure why ...
5 years, 6 months ago (2014-08-08 12:34:08 UTC) #2
Oleksandr
http://codereview.adblockplus.org/6196733759455232/diff/5629499534213120/html/static/js/ieEventListenerPolyfill.js File html/static/js/ieEventListenerPolyfill.js (left): http://codereview.adblockplus.org/6196733759455232/diff/5629499534213120/html/static/js/ieEventListenerPolyfill.js#oldcode1 html/static/js/ieEventListenerPolyfill.js:1: if (typeof window.addEventListener != "function") It actually wasn't working ...
5 years, 6 months ago (2014-08-08 13:11:05 UTC) #3
Felix Dahlke
http://codereview.adblockplus.org/6196733759455232/diff/5649050225344512/html/static/js/ieEventListenerPolyfill.js File html/static/js/ieEventListenerPolyfill.js (right): http://codereview.adblockplus.org/6196733759455232/diff/5649050225344512/html/static/js/ieEventListenerPolyfill.js#newcode17 html/static/js/ieEventListenerPolyfill.js:17: if (Window.addEventListener != "function") Either Window.prototype or window, not ...
5 years, 6 months ago (2014-08-08 13:22:32 UTC) #4
Oleksandr
5 years, 6 months ago (2014-08-08 13:30:45 UTC) #5
Felix Dahlke
http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html/static/js/ieEventListenerPolyfill.js File html/static/js/ieEventListenerPolyfill.js (right): http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html/static/js/ieEventListenerPolyfill.js#newcode17 html/static/js/ieEventListenerPolyfill.js:17: if (typeof Element.prototype.addEventListener != "function") My question remains: Is ...
5 years, 6 months ago (2014-08-08 13:58:35 UTC) #6
Oleksandr
http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html/static/js/ieEventListenerPolyfill.js File html/static/js/ieEventListenerPolyfill.js (right): http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html/static/js/ieEventListenerPolyfill.js#newcode17 html/static/js/ieEventListenerPolyfill.js:17: if (typeof Element.prototype.addEventListener != "function") Oh, I miss understood ...
5 years, 6 months ago (2014-08-08 14:05:17 UTC) #7
Felix Dahlke
http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html/static/js/ieEventListenerPolyfill.js File html/static/js/ieEventListenerPolyfill.js (right): http://codereview.adblockplus.org/6196733759455232/diff/5717271485874176/html/static/js/ieEventListenerPolyfill.js#newcode17 html/static/js/ieEventListenerPolyfill.js:17: if (typeof Element.prototype.addEventListener != "function") On 2014/08/08 14:05:17, Oleksandr ...
5 years, 6 months ago (2014-08-08 14:07:01 UTC) #8
Oleksandr
http://codereview.adblockplus.org/6196733759455232/diff/5733935958982656/html/static/js/ieEventListenerPolyfill.js File html/static/js/ieEventListenerPolyfill.js (right): http://codereview.adblockplus.org/6196733759455232/diff/5733935958982656/html/static/js/ieEventListenerPolyfill.js#newcode19 html/static/js/ieEventListenerPolyfill.js:19: Element.prototype.addEventListener = window.addEventListener; Looks like we've tried every combination ...
5 years, 6 months ago (2014-08-08 14:19:34 UTC) #9
Felix Dahlke
5 years, 6 months ago (2014-08-08 14:21:12 UTC) #10
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 :)
Sign in to reply to this message.

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