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

Issue 29334044: Issue 219 - Donation page is broken in IE8 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 1 month ago by juliandoucette
Modified:
4 years ago
Visibility:
Public.

Description

Issue 219 - Donation page is broken in IE8 It seems this site was generally not developed with IE<9 in mind. This revision implements IE8 support for the donate form in donate.html in roughly as few changes as possible. If we want to support IE8 sitewide, I would recommend introducing a JavaScript library or select polyfills for IE<9 (which can be included without slowing down pageload on other probsers via IFIE comments). I found the following issues in donate.html - IE8 doesn't support addEventListener (use attachEvent and 'on' + event) - IE8 doesn't support DOMContentLoaded (use document readyState and onreadystatechange) - IE8 querySelector doesn't support :checked (use querySelectorAll and a function to return the checked checkbox) - IE8 doesn't support getElementsByClassName (use querySelectorAll) - IE8 event doesn't have target (use event.target or event.srcElement) - IE8 document.head is undefined (use document.getElementsByTagName('head')[0]) - #form should be a form for accessibility (to conform to web standards) - form should submit event instead of click for accessibility (to conform to web standards) - Node.localName was removed from Chrome as of version 46, use tagName (this is out of scope for supporting IE8, but it is related because localName will not be supported by standards in the future) - IE8 buttons submit by default (use event.preventDefault() and event.returnValue = false)

Patch Set 1 #

Total comments: 41

Patch Set 2 : Fixed code style issues, consistency issues, and minor bugs #

Total comments: 2

Patch Set 3 : Added polyfills and fixed spacing #

Total comments: 8

Patch Set 4 : Fixed code style issues #

Patch Set 5 : Uploading diff against tip instead of revision #

Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -44 lines) Patch
M pages/donate.html View 1 2 3 4 6 chunks +53 lines, -42 lines 0 comments Download
A static/js/vendor/ie8.js View 1 2 4 1 chunk +2 lines, -0 lines 0 comments Download
M templates/default.tmpl View 1 2 3 4 2 chunks +7 lines, -2 lines 0 comments Download

Messages

Total messages: 13
juliandoucette
It seems this site was generally not developed with IE<9 in mind. This revision implements ...
4 years, 1 month ago (2016-01-20 11:24:48 UTC) #1
Thomas Greiner
https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html File pages/donate.html (right): https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html#newcode224 pages/donate.html:224: return getCheckedCheckbox(document.querySelectorAll("input[name=currency]")).value; Since you decided to use `<form>` as ...
4 years, 1 month ago (2016-01-22 11:19:32 UTC) #2
juliandoucette
Hey guys, I want to apologize for submitting this with style inconsistencies and thank you ...
4 years, 1 month ago (2016-01-24 23:23:11 UTC) #3
Thomas Greiner
https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html File pages/donate.html (right): https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html#newcode224 pages/donate.html:224: return getCheckedCheckbox(document.querySelectorAll("input[name=currency]")).value; On 2016/01/24 23:23:10, juliandoucette wrote: > On ...
4 years, 1 month ago (2016-01-25 16:56:13 UTC) #4
juliandoucette
https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html File pages/donate.html (right): https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html#newcode224 pages/donate.html:224: return getCheckedCheckbox(document.querySelectorAll("input[name=currency]")).value; On 2016/01/25 16:56:12, Thomas Greiner wrote: > ...
4 years, 1 month ago (2016-01-26 16:37:16 UTC) #5
juliandoucette
On 2016/01/26 16:37:16, juliandoucette wrote: > https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html > File pages/donate.html (right): > > https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html#newcode224 > ...
4 years, 1 month ago (2016-01-26 16:52:31 UTC) #6
Thomas Greiner
https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html File pages/donate.html (right): https://codereview.adblockplus.org/29334044/diff/29334045/pages/donate.html#newcode487 pages/donate.html:487: if (document.addEventListener) { On 2016/01/26 16:37:16, juliandoucette wrote: > ...
4 years ago (2016-01-27 17:20:31 UTC) #7
juliandoucette
> I remembered that we had that problem before and I found the following function ...
4 years ago (2016-01-28 14:02:43 UTC) #8
juliandoucette
https://codereview.adblockplus.org/29334044/diff/29334569/pages/donate.html File pages/donate.html (right): https://codereview.adblockplus.org/29334044/diff/29334569/pages/donate.html#newcode467 pages/donate.html:467: if(event.preventDefault) On 2016/01/27 17:20:31, Thomas Greiner wrote: > Detail: ...
4 years ago (2016-01-28 14:02:54 UTC) #9
Thomas Greiner
https://codereview.adblockplus.org/29334044/diff/29334808/pages/donate.html File pages/donate.html (right): https://codereview.adblockplus.org/29334044/diff/29334808/pages/donate.html#newcode445 pages/donate.html:445: document.getElementById('form-payment').addEventListener("click", onSelectPayment); Detail: Please use double quotes instead of ...
4 years ago (2016-01-28 15:23:37 UTC) #10
juliandoucette
As it turns out, respond.js should be after stylesheets too. IE8.js seems to be the ...
4 years ago (2016-01-28 15:50:45 UTC) #11
juliandoucette
4 years ago (2016-01-28 15:53:53 UTC) #12
Thomas Greiner
4 years ago (2016-01-29 18:07:22 UTC) #13
LGTM
Sign in to reply to this message.

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