Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code
Issues
All Issues

Side by Side Diff: pages/donate.html

Issue 29334044: Issue 219 - Donation page is broken in IE8 (Closed)
Patch Set: Created Jan. 20, 2016, 11:05 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 title=Donate to Adblock Plus 1 title=Donate to Adblock Plus
2 2
3 <head> 3 <head>
4 <style type="text/css"> 4 <style type="text/css">
5 h2 5 h2
6 { 6 {
7 padding-top: 20px; 7 padding-top: 20px;
8 border-top: 1px solid #ccc; 8 border-top: 1px solid #ccc;
9 } 9 }
10 10
(...skipping 203 matching lines...) Expand 10 before | Expand all | Expand 10 after
214 { 214 {
215 var textAttr = ("textContent" in document.documentElement) ? "textContent" : " innerText"; 215 var textAttr = ("textContent" in document.documentElement) ? "textContent" : " innerText";
216 216
217 var formValues = { 217 var formValues = {
218 name: null 218 name: null
219 }; 219 };
220 220
221 var currencyValue = { 221 var currencyValue = {
222 get: function() 222 get: function()
223 { 223 {
224 return document.querySelector("input[name=currency]:checked").value; 224 return getCheckedCheckbox(document.querySelectorAll("input[name=currency]" )).value;
Thomas Greiner 2016/01/22 11:19:32 Since you decided to use `<form>` as a wrapper you
juliandoucette 2016/01/24 23:23:10 This is not true in IE8, unfortunately.
Thomas Greiner 2016/01/25 16:56:12 That's a real pity. In that case that function see
juliandoucette 2016/01/26 16:37:16 Acknowledged.
225 } 225 }
226 }; 226 };
227 227
228 var priceValue = { 228 var priceValue = {
229 get: function() 229 get: function()
230 { 230 {
231 var value = document.querySelector("input[name=price]:checked").value; 231 var value = getCheckedCheckbox(document.querySelectorAll("input[name=price ]")).value;
232 if (value == 0) 232 if (value == 0)
233 value = document.getElementById("price-other").value >> 0; 233 value = document.getElementById("price-other").value >> 0;
234 return value; 234 return value;
235 }, 235 },
236 min: function() 236 min: function()
237 { 237 {
238 return parseInt(document.getElementById("price-other").getAttribute("min") , 10); 238 return parseInt(document.getElementById("price-other").getAttribute("min") , 10);
239 }, 239 },
240 verify: function() 240 verify: function()
241 { 241 {
242 var selected = document.querySelector("input[name=price]:checked"); 242 var selected = getCheckedCheckbox(document.querySelectorAll("input[name=pr ice]"));
243 if (!selected) 243 if (!selected)
244 return "Please select an amount"; 244 return "Please select an amount";
245 if (this.get() < this.min()) 245 if (this.get() < this.min())
246 return "Please select a valid amount"; 246 return "Please select a valid amount";
247 return null; 247 return null;
248 } 248 }
249 }; 249 };
250 250
251 var langValue = { 251 var langValue = {
252 get: function(langs) 252 get: function(langs)
(...skipping 131 matching lines...) Expand 10 before | Expand all | Expand 10 after
384 for (var i in fields) 384 for (var i in fields)
385 { 385 {
386 var field = document.createElement("input"); 386 var field = document.createElement("input");
387 field.name = i; 387 field.name = i;
388 field.value = fields[i]; 388 field.value = fields[i];
389 form.appendChild(field); 389 form.appendChild(field);
390 } 390 }
391 } 391 }
392 392
393 // To submit a form in Firefox it needs to be part of the DOM 393 // To submit a form in Firefox it needs to be part of the DOM
394 document.head.appendChild(form); 394 var docHead = document.getElementsByTagName('head')[0]
395 docHead.appendChild(form);
395 form.submit(); 396 form.submit();
396 document.head.removeChild(form); 397 docHead.removeChild(form);
397 } 398 }
398 399
399 function getText(id) 400 function getText(id)
400 { 401 {
401 var element = document.getElementById(id); 402 var element = document.getElementById(id);
402 return element[textAttr]; 403 return element[textAttr];
403 } 404 }
404 405
405 function ensureMinPrice() 406 function ensureMinPrice()
406 { 407 {
407 // Credit card payments have to be above 1 EUR, meaning at least 2 USD. 408 // Credit card payments have to be above 1 EUR, meaning at least 2 USD.
408 var provider = document.getElementById("form").className; 409 var provider = document.getElementById("form").className;
409 var minPrice = (provider == "credit-card" && currencyValue.get() == "USD" ? 2 : 1); 410 var minPrice = (provider == "credit-card" && currencyValue.get() == "USD" ? 2 : 1);
410 var priceOther = document.getElementById("price-other"); 411 var priceOther = document.getElementById("price-other");
411 priceOther.setAttribute("min", minPrice); 412 priceOther.setAttribute("min", minPrice);
412 if (priceOther.value >> 0 < minPrice) 413 if (priceOther.value >> 0 < minPrice)
413 priceOther.value = minPrice; 414 priceOther.value = minPrice;
414 } 415 }
416
417 function getCheckedCheckbox (checkboxes)
Thomas Greiner 2016/01/22 11:19:31 I noticed a couple of inconsistencies with our cod
juliandoucette 2016/01/24 23:23:10 I have to apologize for this. I should not have su
Thomas Greiner 2016/01/25 16:56:12 No worries. That's one reason why we have code rev
juliandoucette 2016/01/26 16:37:15 Done.
418 {
419 var checkboxLength = checkboxes.length;
420 while(checkboxLength--) {
Thomas Greiner 2016/01/22 11:19:31 Detail: This is inconsistent with our coding style
juliandoucette 2016/01/26 16:37:15 Done.
421 if (checkboxes[checkboxLength].checked) {
422 return checkboxes[checkboxLength];
423 }
424 }
425 }
415 426
416 function updateCurrency() 427 function updateCurrency()
417 { 428 {
418 var checkbox = document.querySelector("input[name=currency]:checked"); 429 var currencyField = document.getElementById('form-currency');
Thomas Greiner 2016/01/22 11:19:31 Detail: This is inconsistent with our coding style
juliandoucette 2016/01/26 16:37:16 Done.
419 if (!checkbox) 430 var selectedCurrency = getCheckedCheckbox(
431 currencyField.querySelectorAll("input[name=currency]")
432 );
433 if (typeof selectedCurrency === 'undefined') {
Thomas Greiner 2016/01/22 11:19:31 Detail: This is inconsistent with our coding style
juliandoucette 2016/01/26 16:37:16 Done.
420 return; 434 return;
421 435 }
422 var currencyLabel = checkbox.parentNode[textAttr]; 436 var currencyLabel = selectedCurrency.parentNode.innerText;
Thomas Greiner 2016/01/22 11:19:32 `innerText` is not supported by most Firefox versi
juliandoucette 2016/01/24 23:23:10 Good catch.
juliandoucette 2016/01/26 16:37:16 Done.
423 if (!currencyLabel) 437 var currencyLabels = document.querySelectorAll('.currency-label');
424 return; 438 for (var i = 0, ii = currencyLabels.length; i < ii; i++)
Thomas Greiner 2016/01/22 11:19:31 Detail: This is premature optimization. The perfor
juliandoucette 2016/01/24 23:23:10 Stylistic habit. I see your point.
juliandoucette 2016/01/26 16:37:16 Done.
425 439 currencyLabels[i][textAttr] = currencyLabel;
426 var elements = document.getElementsByClassName("currency-label");
427 for (var i = 0; i < elements.length; i++)
428 elements[i][textAttr] = currencyLabel;
429 ensureMinPrice(); 440 ensureMinPrice();
430 } 441 }
442
443 function addEventsBySelector (query, event, callback, bubble, base)
Thomas Greiner 2016/01/22 11:19:30 Why do you specify the "bubble" and "base" paramet
Thomas Greiner 2016/01/22 11:19:31 Detail: You're not adding events but event listene
juliandoucette 2016/01/24 23:23:09 I intended to use them when I wrote this function.
juliandoucette 2016/01/24 23:23:10 I chose "addEvent" instead of "addListeners" to be
juliandoucette 2016/01/26 16:37:16 Done.
444 {
445 base = base || document;
446 bubble = bubble || false;
447 var query = base.querySelectorAll(query);
Thomas Greiner 2016/01/22 11:19:31 `query` has already been declared so no need for `
juliandoucette 2016/01/24 23:23:10 Good catch.
juliandoucette 2016/01/26 16:37:15 Done.
448 var queryIndex = query.length;
449 //fallback to attachEvent for IE8
450 var addListenerFunction = typeof document.addEventListener === 'function' ?
451 'addEventListener' :
452 'attachEvent';
453 if (addListenerFunction === 'attachEvent') {
454 event = 'on' + event;
455 }
456 while(queryIndex--) {
457 query[queryIndex][addListenerFunction](event, callback, bubble);
458 }
459 }
431 460
432 document.addEventListener("DOMContentLoaded", function() 461 function initializeDonationForm ()
433 { 462 {
434 formValues.name = getText("i18n_name"); 463 formValues.name = getText("i18n_name");
435 464
436 document.getElementById("payment").addEventListener("click", function(event) 465 addEventsBySelector('button[name="payment"]', 'click', onSelectPayment);
Thomas Greiner 2016/01/22 11:19:32 What's the reason for this change? Did you see any
juliandoucette 2016/01/24 23:23:09 - attachEvent doesn't support bubbling (Therefore
Thomas Greiner 2016/01/25 16:56:12 Thanks for the explanation. I just wanted to make
juliandoucette 2016/01/26 16:37:15 Acknowledged.
437 { 466 addEventsBySelector('input[name="currency"]', 'click', updateCurrency);
438 if (event.target.localName != "button") 467 addEventsBySelector('#form', 'submit', onSubmitDonation);
439 return; 468
440
441 document.getElementById("form").className = event.target.className;
442 ensureMinPrice();
443 }, true);
444
445 document.getElementById("form-currency").addEventListener("click", function( event)
446 {
447 if (event.target.localName == "input")
448 updateCurrency();
449 }, true);
450 updateCurrency(); 469 updateCurrency();
451 470 }
452 document.getElementById("donate").addEventListener("click", function(event) 471
453 { 472 function onSelectPayment (event)
454 var provider = providers[document.getElementById("form").className]; 473 {
474 event.preventDefault ? event.preventDefault() : (event.returnValue = false);
Thomas Greiner 2016/01/22 11:19:30 Detail: We usually don't write single-line stateme
juliandoucette 2016/01/26 16:37:15 Done.
475 document.getElementById("form").className = (event.target || event.srcElemen t).className;
476 ensureMinPrice();
477 }
478
479 function onSubmitDonation (event)
480 {
481 event.preventDefault ? event.preventDefault() : (event.returnValue = false);
482 var provider = providers[document.getElementById("form").className];
455 if (provider) 483 if (provider)
Thomas Greiner 2016/01/22 11:19:31 Detail: The indentation here is inconsistent with
juliandoucette 2016/01/26 16:37:15 Done.
456 provider(); 484 provider();
457 }, false); 485 }
458 }, false); 486
487 if (document.addEventListener) {
Thomas Greiner 2016/01/22 11:19:31 Since you have that fallback in two different plac
juliandoucette 2016/01/24 23:23:09 I agree. I wasn't sure if adding a polyfill for th
Thomas Greiner 2016/01/25 16:56:12 Generally, it's about maintenance effort. Checking
juliandoucette 2016/01/26 16:37:16 Acknowledged.
Thomas Greiner 2016/01/27 17:20:31 I remembered that we had that problem before and I
488 document.addEventListener('DOMContentLoaded', initializeDonationForm, false) ;
489 } else {
490 document.attachEvent('onreadystatechange', initializeDonationForm);
491 }
492
459 })(); 493 })();
460 494
461 // --> 495 // -->
462 </script> 496 </script>
463 </head> 497 </head>
464 <span class="hidden" id="i18n_name">{{s1 Adblock Plus Contribution}}</span> 498 <span class="hidden" id="i18n_name">{{s1 Adblock Plus Contribution}}</span>
465 499
466 <p> 500 <p>
467 {{s2 Adblock Plus is - and will always be - free. Our mission is to provide us ers with tools that allow them to gain control over their Internet browsing and protect them from unwanted and malicious elements. 501 {{s2 Adblock Plus is - and will always be - free. Our mission is to provide us ers with tools that allow them to gain control over their Internet browsing and protect them from unwanted and malicious elements.
468 Your donation directly helps the development of Adblock Plus.}} 502 Your donation directly helps the development of Adblock Plus.}}
469 </p> 503 </p>
470 504
471 <div class="paypal" id="form"> 505 <form class="paypal" id="form">
472 <p id="payment"> 506 <p id="payment">
473 <strong>{{s3 Select your payment method}}</strong> 507 <strong>{{s3 Select your payment method}}</strong>
474 508
475 <button class="paypal">{{s4 PayPal}}</button> 509 <button class="paypal" value="paypal" name="payment">{{s4 PayPal}}</button>
476 <button class="credit-card">{{s5 Credit Card}}</button> 510 <button class="credit-card" value="credit-card" name="payment">{{s5 Credit C ard}}</button>
477 </p> 511 </p>
478 512
479 <div id="form-fields"> 513 <div id="form-fields">
480 <p id="form-currency"> 514 <p id="form-currency">
481 <strong>{{s6 Choose your currency}}</strong> 515 <strong>{{s6 Choose your currency}}</strong>
482 516
483 <label> 517 <label>
484 <input checked="checked" name="currency" type="radio" value="USD"> 518 <input checked="checked" name="currency" type="radio" value="USD">
485 $ 519 $
486 </label> 520 </label>
(...skipping 46 matching lines...) Expand 10 before | Expand all | Expand 10 after
533 {{s11 You can receive a refund within 120 days of payment. Please send a n email to <a href="mailto:info@adblockplus.org"><fix>info@adblockplus.org</fix> </a>.}} 567 {{s11 You can receive a refund within 120 days of payment. Please send a n email to <a href="mailto:info@adblockplus.org"><fix>info@adblockplus.org</fix> </a>.}}
534 </div> 568 </div>
535 <div id="recurrent-cancellation"> 569 <div id="recurrent-cancellation">
536 * {{s12 To cancel your monthly payment, click the link below that corres ponds to how you donated:}} 570 * {{s12 To cancel your monthly payment, click the link below that corres ponds to how you donated:}}
537 <ul> 571 <ul>
538 <li><a href="https://www.paypal.com/cgi-bin/webscr?cmd=_manage-paylist " target="_blank">{{s13 I donated with PayPal}}</a></li> 572 <li><a href="https://www.paypal.com/cgi-bin/webscr?cmd=_manage-paylist " target="_blank">{{s13 I donated with PayPal}}</a></li>
539 </ul> 573 </ul>
540 </div> 574 </div>
541 </div> 575 </div>
542 </div> 576 </div>
543 </div> 577 </form>
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld