From 3320ecf41881826704823445bbb9a9fd60aa7141 Mon Sep 17 00:00:00 2001
From: Matjaz Gregoric <mtyaka@gmail.com>
Date: Mon, 2 Nov 2015 09:47:09 +0100
Subject: [PATCH] Move unenroll messages into separate data attributes.

Having the messeges embedded into onclick attributes was fragile because
it could break if the translated string contained special characters
such as single quotes.

Rather than trying to escape the special characters inside the onclick handler,
move the translatable strings into separate h-escaped data attributes and interpolate
the values in the (already existing) jQuery click handler rather than in the
inline onclick handler.

This makes the javascript code more consistent as well as fixes the problem
where the popup would break if translation included single quotes.
---
 lms/static/js/dashboard/legacy.js             | 19 ++++--
 .../dashboard/_dashboard_course_listing.html  | 68 +++++++------------
 2 files changed, 37 insertions(+), 50 deletions(-)

diff --git a/lms/static/js/dashboard/legacy.js b/lms/static/js/dashboard/legacy.js
index 998944029f..81bb1dacfb 100644
--- a/lms/static/js/dashboard/legacy.js
+++ b/lms/static/js/dashboard/legacy.js
@@ -7,7 +7,7 @@
  */
  var edx = edx || {};
 
-(function($, gettext, Logger, accessibleModal) {
+(function($, gettext, Logger, accessibleModal, interpolate) {
     'use strict';
 
     edx.dashboard = edx.dashboard || {};
@@ -126,9 +126,18 @@
         });
 
         $(".action-unenroll").click(function(event) {
-            $("#unenroll_course_id").val( $(event.target).data("course-id") );
-            $("#unenroll_course_number").text( $(event.target).data("course-number") );
-            $("#unenroll_course_name").text( $(event.target).data("course-name") );
+            var element = $(event.target);
+            var track_info = element.data("track-info");
+            var course_number = element.data("course-number");
+            var course_name = element.data("course-name");
+            var cert_name_long = element.data("cert-name-long");
+            $('#track-info').html(interpolate(track_info, {
+                course_number: "<span id='unenroll_course_number'>" + course_number + "</span>",
+                course_name: "<span id='unenroll_course_name'>" + course_name + "</span>",
+                cert_name_long: "<span id='unenroll_cert_name'>" + cert_name_long + "</span>"
+            }, true));
+            $('#refund-info').html( element.data("refund-info") );
+            $("#unenroll_course_id").val( element.data("course-id") );
         });
 
         $('#unenroll_form').on('ajax:complete', function(event, xhr) {
@@ -195,4 +204,4 @@
         });
     };
 
-})(jQuery, gettext, Logger, accessible_modal);
+})(jQuery, gettext, Logger, accessible_modal, interpolate); // jshint undef:false
diff --git a/lms/templates/dashboard/_dashboard_course_listing.html b/lms/templates/dashboard/_dashboard_course_listing.html
index 59db2b4213..50d45e7ae2 100644
--- a/lms/templates/dashboard/_dashboard_course_listing.html
+++ b/lms/templates/dashboard/_dashboard_course_listing.html
@@ -181,83 +181,70 @@ from student.helpers import (
                   % if cert_status.get('status') not in DISABLE_UNENROLL_CERT_STATES:
                     <li class="actions-item" id="actions-item-unenroll-${dashboard_index}">
                     % if is_paid_course and show_refund_option:
-                      ## Translators: The course name will be added to the end of this sentence.
                       % if not is_course_blocked:
-                      <a href="#unenroll-modal" class="action action-unenroll" rel="leanModal" data-course-id="${course_overview.id | h}" data-course_overview-number="${course_overview.number | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message('${_("Are you sure you want to unenroll from the purchased course %(course_name)s (%(course_number)s)?")}', '${_("You will be refunded the amount you paid.")}')">
+                      <a href="#unenroll-modal" class="action action-unenroll" rel="leanModal" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
+                         data-track-info="${_("Are you sure you want to unenroll from the purchased course %(course_name)s (%(course_number)s)?") | h}"
+                         data-refund-info="${_("You will be refunded the amount you paid.") | h}">
                         ${_('Unenroll')}
                       </a>
                       % else:
                       <a class="action action-unenroll is-disabled" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message('${_("Are you sure you want to unenroll from the purchased course %(course_name)s (%(course_number)s)?")}', '${_("You will be refunded the amount you paid.")}')">
+                         data-track-info="${_("Are you sure you want to unenroll from the purchased course %(course_name)s (%(course_number)s)?") | h}"
+                         data-refund-info="${_("You will be refunded the amount you paid.") | h}">
                         ${_('Unenroll')}
                       </a>
                       % endif
                     % elif is_paid_course and not show_refund_option:
-                      ## Translators: The course's name will be added to the end of this sentence.
                       % if not is_course_blocked:
                       <a href="#unenroll-modal" class="action action-unenroll" rel="leanModal" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message('${_("Are you sure you want to unenroll from the purchased course %(course_name)s (%(course_number)s)?")}', '${_("You will not be refunded the amount you paid.")}')">
+                         data-track-info="${_("Are you sure you want to unenroll from the purchased course %(course_name)s (%(course_number)s)?") | h}"
+                         data-refund-info="${_("You will not be refunded the amount you paid.") | h}">
                         ${_('Unenroll')}
                       </a>
                       % else:
                       <a class="action action-unenroll is-disabled" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message('${_("Are you sure you want to unenroll from the purchased course %(course_name)s (%(course_number)s)?")}', '${_("You will not be refunded the amount you paid.")}')">
+                         data-track-info="${_("Are you sure you want to unenroll from the purchased course %(course_name)s (%(course_number)s)?") | h}"
+                         data-refund-info="${_("You will not be refunded the amount you paid.") | h}">
                         ${_('Unenroll')}
                       </a>
                       % endif
                     % elif enrollment.mode != "verified":
-                      ## Translators: The course's name will be added to the end of this sentence.
                       % if not is_course_blocked:
                       <a href="#unenroll-modal" class="action action-unenroll" rel="leanModal" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message('${_("Are you sure you want to unenroll from %(course_name)s (%(course_number)s)?")}', '')">
+                         data-track-info="${_("Are you sure you want to unenroll from %(course_name)s (%(course_number)s)?") | h}">
                         ${_('Unenroll')}
                       </a>
                       % else:
                       <a class="action action-unenroll is-disabled" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message('${_("Are you sure you want to unenroll from %(course_name)s (%(course_number)s)?")}', '')">
+                         data-track-info="${_("Are you sure you want to unenroll from %(course_name)s (%(course_number)s)?") | h}">
                         ${_('Unenroll')}
                       </a>
                       % endif
                     % elif show_refund_option:
-                      ## Translators: The course's name will be added to the end of this sentence.
                       % if not is_course_blocked:
-                      <a href="#unenroll-modal" class="action action-unenroll" rel="leanModal" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message(
-                                      '${_("Are you sure you want to unenroll from the verified %(cert_name_long)s track of %(course_name)s (%(course_number)s)?")}',
-                                      '${_("You will be refunded the amount you paid.")}'
-                                  )"
-                      >
+                      <a href="#unenroll-modal" class="action action-unenroll" rel="leanModal" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}" data-cert-name-long="${cert_name_long | h}"
+                         data-track-info="${_("Are you sure you want to unenroll from the verified %(cert_name_long)s track of %(course_name)s (%(course_number)s)?") | h}"
+                         data-refund-info="${_("You will be refunded the amount you paid.") | h}">
                         ${_('Unenroll')}
                       </a>
                       % else:
-                      <a class="action action-unenroll is-disabled" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message(
-                                      '${_("Are you sure you want to unenroll from the verified %(cert_name_long)s track of %(course_name)s (%(course_number)s)?")}',
-                                      '${_("You will be refunded the amount you paid.")}'
-                                  )"
-                      >
+                      <a class="action action-unenroll is-disabled" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}" data-cert-name-long="${cert_name_long | h}"
+                         data-track-info="${_("Are you sure you want to unenroll from the verified %(cert_name_long)s track of %(course_name)s (%(course_number)s)?") | h}"
+                         data-refund-info="${_("You will be refunded the amount you paid.") | h}">
                         ${_('Unenroll')}
                       </a>
                       % endif
                     % else:
-                      ## Translators: The course's name will be added to the end of this sentence.
                       % if not is_course_blocked:
-                      <a href="#unenroll-modal" class="action action-unenroll" rel="leanModal" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message(
-                                      '${_("Are you sure you want to unenroll from the verified %(cert_name_long)s track of %(course_name)s (%(course_number)s)?")}',
-                                      '${_("The refund deadline for this course has passed, so you will not receive a refund.")}'
-                                  )"
-                      >
+                      <a href="#unenroll-modal" class="action action-unenroll" rel="leanModal" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}" data-cert-name-long="${cert_name_long | h}"
+                         data-track-info="${_("Are you sure you want to unenroll from the verified %(cert_name_long)s track of %(course_name)s (%(course_number)s)?") | h}"
+                         data-refund-info="${_("The refund deadline for this course has passed, so you will not receive a refund.") | h}">
                         ${_('Unenroll')}
                       </a>
                       % else:
-                      <a class="action action-unenroll is-disabled" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}"
-                         onclick="set_unenroll_message(
-                                      '${_("Are you sure you want to unenroll from the verified %(cert_name_long)s track of %(course_name)s (%(course_number)s)?")}',
-                                      '${_("The refund deadline for this course has passed, so you will not receive a refund.")}'
-                                  )"
-                      >
+                      <a class="action action-unenroll is-disabled" data-course-id="${course_overview.id | h}" data-course-number="${course_overview.number | h}" data-course-name="${course_overview.display_name_with_default | h}" data-dashboard-index="${dashboard_index}" data-cert-name-long="${cert_name_long | h}"
+                         data-track-info="${_("Are you sure you want to unenroll from the verified %(cert_name_long)s track of %(course_name)s (%(course_number)s)?") | h}"
+                         data-refund-info="${_("The refund deadline for this course has passed, so you will not receive a refund.") | h}">
                         ${_('Unenroll')}
                       </a>
                       % endif
@@ -403,13 +390,4 @@ from student.helpers import (
                    });
                }
            });
-
-           function set_unenroll_message(track_info, refund_info) {
-               document.getElementById('track-info').innerHTML = interpolate(track_info, {
-                  course_number: "<span id='unenroll_course_number'></span>",
-                  course_name: "<span id='unenroll_course_name'></span>",
-                  cert_name_long: "${cert_name_long | h}"
-               }, true);
-               document.getElementById('refund-info').innerHTML = refund_info;
-           }
 </script>