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

Delta Between Two Patch Sets: pages/donate.html

Issue 29334044: Issue 219 - Donation page is broken in IE8 (Closed)
Left Patch Set: Created Jan. 20, 2016, 11:05 a.m.
Right Patch Set: Uploading diff against tip instead of revision Created Jan. 28, 2016, 3:53 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | static/js/vendor/ie8.js » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
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 getCheckedCheckbox(document.querySelectorAll("input[name=currency]" )).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 = getCheckedCheckbox(document.querySelectorAll("input[name=price ]")).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;
(...skipping 149 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 var docHead = document.getElementsByTagName('head')[0] 394 var documentHead = document.getElementsByTagName("head")[0]
395 docHead.appendChild(form); 395 documentHead.appendChild(form);
396 form.submit(); 396 form.submit();
397 docHead.removeChild(form); 397 documentHead.removeChild(form);
398 } 398 }
399 399
400 function getText(id) 400 function getText(id)
401 { 401 {
402 var element = document.getElementById(id); 402 var element = document.getElementById(id);
403 return element[textAttr]; 403 return element[textAttr];
404 } 404 }
405 405
406 function ensureMinPrice() 406 function ensureMinPrice()
407 { 407 {
408 // 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.
409 var provider = document.getElementById("form").className; 409 var provider = document.getElementById("form").className;
410 var minPrice = (provider == "credit-card" && currencyValue.get() == "USD" ? 2 : 1); 410 var minPrice = (provider == "credit-card" && currencyValue.get() == "USD" ? 2 : 1);
411 var priceOther = document.getElementById("price-other"); 411 var priceOther = document.getElementById("price-other");
412 priceOther.setAttribute("min", minPrice); 412 priceOther.setAttribute("min", minPrice);
413 if (priceOther.value >> 0 < minPrice) 413 if (priceOther.value >> 0 < minPrice)
414 priceOther.value = minPrice; 414 priceOther.value = minPrice;
415 } 415 }
416 416
417 function getCheckedCheckbox (checkboxes) 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 { 418 {
419 var checkboxLength = checkboxes.length; 419 var checkboxLength = checkboxes.length;
420 while(checkboxLength--) { 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) { 421 {
422 if (checkboxes[checkboxLength].checked)
422 return checkboxes[checkboxLength]; 423 return checkboxes[checkboxLength];
423 }
424 } 424 }
425 } 425 }
426 426
427 function updateCurrency() 427 function updateCurrency()
428 { 428 {
429 var currencyField = document.getElementById('form-currency'); 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.
430 var selectedCurrency = getCheckedCheckbox( 430 var selectedCurrency = getCheckedCheckbox(
431 currencyField.querySelectorAll("input[name=currency]") 431 currencyField.querySelectorAll("input[name=currency]")
432 ); 432 );
433 if (typeof selectedCurrency === 'undefined') { 433 if (!selectedCurrency)
Thomas Greiner 2016/01/22 11:19:31 Detail: This is inconsistent with our coding style
juliandoucette 2016/01/26 16:37:16 Done.
434 return; 434 return;
435 } 435 var currencyLabel = selectedCurrency.parentNode[textAttr];
436 var currencyLabel = selectedCurrency.parentNode.innerText; 436 var currencyLabels = document.querySelectorAll(".currency-label");
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.
437 var currencyLabels = document.querySelectorAll('.currency-label'); 437 for (var i = 0; i < currencyLabels.length; i++)
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.
439 currencyLabels[i][textAttr] = currencyLabel; 438 currencyLabels[i][textAttr] = currencyLabel;
440 ensureMinPrice(); 439 ensureMinPrice();
441 } 440 }
442 441
443 function addEventsBySelector (query, event, callback, bubble, base) 442 function initializeDonationForm()
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 }
460
461 function initializeDonationForm ()
462 { 443 {
463 formValues.name = getText("i18n_name"); 444 formValues.name = getText("i18n_name");
464 445 document.getElementById("form-payment").addEventListener("click", onSelectPa yment, true);
465 addEventsBySelector('button[name="payment"]', 'click', onSelectPayment); 446 document.getElementById("form-currency").addEventListener("click", updateCur rency, true);
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.
466 addEventsBySelector('input[name="currency"]', 'click', updateCurrency); 447 document.getElementById("form").addEventListener("submit", onSubmitDonation, false);
467 addEventsBySelector('#form', 'submit', onSubmitDonation);
468
469 updateCurrency(); 448 updateCurrency();
470 } 449 }
471 450
472 function onSelectPayment (event) 451 function onSelectPayment(event)
473 { 452 {
474 event.preventDefault ? event.preventDefault() : (event.returnValue = false); 453 if (event.target.tagName != "BUTTON")
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; 454 return;
455 event.preventDefault();
456 document.getElementById("form").className = event.target.className;
476 ensureMinPrice(); 457 ensureMinPrice();
477 } 458 }
478 459
479 function onSubmitDonation (event) 460 function onSubmitDonation(event)
480 { 461 {
481 event.preventDefault ? event.preventDefault() : (event.returnValue = false); 462 event.preventDefault();
482 var provider = providers[document.getElementById("form").className]; 463 var provider = providers[document.getElementById("form").className];
483 if (provider) 464 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.
484 provider(); 465 provider();
485 } 466 }
486 467
487 if (document.addEventListener) { 468 document.addEventListener("DOMContentLoaded", initializeDonationForm, false);
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 469
493 })(); 470 })();
494 471
495 // --> 472 // -->
496 </script> 473 </script>
497 </head> 474 </head>
498 <span class="hidden" id="i18n_name">{{s1 Adblock Plus Contribution}}</span> 475 <span class="hidden" id="i18n_name">{{s1 Adblock Plus Contribution}}</span>
499 476
500 <p> 477 <p>
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. 478 {{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.
502 Your donation directly helps the development of Adblock Plus.}} 479 Your donation directly helps the development of Adblock Plus.}}
503 </p> 480 </p>
504 481
505 <form class="paypal" id="form"> 482 <form class="paypal" id="form">
506 <p id="payment"> 483 <p id="form-payment">
507 <strong>{{s3 Select your payment method}}</strong> 484 <strong>{{s3 Select your payment method}}</strong>
508 485
509 <button class="paypal" value="paypal" name="payment">{{s4 PayPal}}</button> 486 <button class="paypal" value="paypal" name="payment">{{s4 PayPal}}</button>
510 <button class="credit-card" value="credit-card" name="payment">{{s5 Credit C ard}}</button> 487 <button class="credit-card" value="credit-card" name="payment">{{s5 Credit C ard}}</button>
511 </p> 488 </p>
512 489
513 <div id="form-fields"> 490 <div id="form-fields">
514 <p id="form-currency"> 491 <p id="form-currency">
515 <strong>{{s6 Choose your currency}}</strong> 492 <strong>{{s6 Choose your currency}}</strong>
516 493
(...skipping 51 matching lines...) Expand 10 before | Expand all | Expand 10 after
568 </div> 545 </div>
569 <div id="recurrent-cancellation"> 546 <div id="recurrent-cancellation">
570 * {{s12 To cancel your monthly payment, click the link below that corres ponds to how you donated:}} 547 * {{s12 To cancel your monthly payment, click the link below that corres ponds to how you donated:}}
571 <ul> 548 <ul>
572 <li><a href="https://www.paypal.com/cgi-bin/webscr?cmd=_manage-paylist " target="_blank">{{s13 I donated with PayPal}}</a></li> 549 <li><a href="https://www.paypal.com/cgi-bin/webscr?cmd=_manage-paylist " target="_blank">{{s13 I donated with PayPal}}</a></li>
573 </ul> 550 </ul>
574 </div> 551 </div>
575 </div> 552 </div>
576 </div> 553 </div>
577 </form> 554 </form>
LEFTRIGHT

Powered by Google App Engine
This is Rietveld