Merge pull request #17069 from edx/adeel/Learner_3110_fix_invalid_certificate_page

Fix invalid certificate page for a11y and translation.
2018-01-25 11:48:33 +01:00
parent d21a098280 d9d4ae5535
commit 6477482c7e
13 changed files with 90 additions and 26 deletions
--- a/lms/djangoapps/certificates/tests/test_webview_views.py
+++ b/lms/djangoapps/certificates/tests/test_webview_views.py
@@ -11,6 +11,7 @@ from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.test.client import Client, RequestFactory
 from django.test.utils import override_settings
+from django.utils import translation
 from mock import patch

 import ddt
@@ -41,6 +42,7 @@ from nose.plugins.attrib import attr
 from openedx.core.djangoapps.certificates.config import waffle
 from openedx.core.djangoapps.dark_lang.models import DarkLangConfig
 from openedx.core.lib.tests.assertions.events import assert_event_matches
+from openedx.core.djangolib.js_utils import js_escaped_string
 from student.roles import CourseStaffRole
 from student.tests.factories import CourseEnrollmentFactory, UserFactory
 from track.tests import EventTrackingTestCase
@@ -273,8 +275,8 @@ class CertificatesViewsTests(CommonCertificatesTestCase):
            ('pfCertificationUrl', self.request.build_absolute_uri(test_url),),
        ])
        self.assertIn(
-            self.linkedin_url.format(params=urlencode(params)),
-            response.content
+            js_escaped_string(self.linkedin_url.format(params=urlencode(params))),
+            response.content.decode('utf-8')
        )

    @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
@@ -296,7 +298,7 @@ class CertificatesViewsTests(CommonCertificatesTestCase):
            ('pfCertificationUrl', 'http://' + settings.MICROSITE_TEST_HOSTNAME + test_url,),
        ])
        self.assertIn(
-            self.linkedin_url.format(params=urlencode(params)),
+            js_escaped_string(self.linkedin_url.format(params=urlencode(params))),
            response.content
        )

@@ -655,6 +657,29 @@ class CertificatesViewsTests(CommonCertificatesTestCase):
        self.assertIn("Cannot Find Certificate", response.content)
        self.assertIn("We cannot find a certificate with this URL or ID number.", response.content)

+    @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
+    def test_html_lang_attribute_is_dynamic_for_invalid_certificate_html_view(self):
+        """
+        Tests that Certificate HTML Web View's lang attribute is based on user language.
+        """
+        self._add_course_certificates(count=1, signatory_count=2)
+        test_url = get_certificate_url(
+            user_id=self.user.id,
+            course_id=unicode(self.course.id)
+        )
+
+        self.cert.invalidate()
+
+        user_language = 'fr'
+        self.client.cookies[settings.LANGUAGE_COOKIE] = user_language
+        response = self.client.get(test_url)
+        self.assertIn('<html class="no-js" lang="fr">', response.content)
+
+        user_language = 'ar'
+        self.client.cookies[settings.LANGUAGE_COOKIE] = user_language
+        response = self.client.get(test_url)
+        self.assertIn('<html class="no-js" lang="ar">', response.content)
+
    @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
    def test_html_view_for_non_viewable_certificate_and_for_student_user(self):
        """
@@ -782,6 +807,34 @@ class CertificatesViewsTests(CommonCertificatesTestCase):
        self.assertNotIn('Signatory_Name 0', response.content)
        self.assertNotIn('Signatory_Title 0', response.content)

+    @override_settings(FEATURES=FEATURES_WITH_CERTS_ENABLED)
+    def test_render_html_view_is_html_escaped(self):
+        test_certificates = [
+            {
+                'id': 0,
+                'name': 'Certificate Name',
+                'description': '<script>Description</script>',
+                'course_title': '<script>course_title</script>',
+                'org_logo_path': '/t4x/orgX/testX/asset/org-logo-1.png',
+                'signatories': [],
+                'version': 1,
+                'is_active': True
+            }
+        ]
+
+        self.course.certificates = {'certificates': test_certificates}
+        self.course.cert_html_view_enabled = True
+        self.course.save()
+        self.store.update_item(self.course, self.user.id)
+
+        test_url = get_certificate_url(
+            user_id=self.user.id,
+            course_id=unicode(self.course.id)
+        )
+        response = self.client.get(test_url)
+        self.assertNotIn('<script>', response.content)
+        self.assertIn('&lt;script&gt;course_title&lt;/script&gt;', response.content)
+
    @override_settings(FEATURES=FEATURES_WITH_CERTS_DISABLED)
    def test_render_html_view_disabled_feature_flag_returns_static_url(self):
        test_url = get_certificate_url(
--- a/lms/djangoapps/certificates/views/webview.py
+++ b/lms/djangoapps/certificates/views/webview.py
@@ -172,8 +172,7 @@ def _update_context_with_basic_info(context, course_id, platform_name, configura
    # in the browser title bar when a requested certificate is not found or recognized
    context['document_title'] = _("Invalid Certificate")

-    # Translators: The &amp; characters represent an ampersand character and can be ignored
-    context['company_tos_urltext'] = _("Terms of Service &amp; Honor Code")
+    context['company_tos_urltext'] = _("Terms of Service & Honor Code")

    # Translators: A 'Privacy Policy' is a legal document/statement describing a website's use of personal information
    context['company_privacy_urltext'] = _("Privacy Policy")
--- a/lms/templates/certificates/_about-accomplishments.html
+++ b/lms/templates/certificates/_about-accomplishments.html
@@ -1,3 +1,4 @@
+<%page expression_filter="h"/>
 <section class="about-item about-accomplishments">
    <h2 class="about-title hd-4">${accomplishment_copy_about}</h2>

--- a/lms/templates/certificates/_about-edx.html
+++ b/lms/templates/certificates/_about-edx.html
@@ -1,3 +1,4 @@
+<%page expression_filter="h"/>
 <%! from django.utils.translation import ugettext as _ %>
 <section class="about-item about-edx">
    <h2 class="about-edx-title hd-4">${company_about_title}</h2>
--- a/lms/templates/certificates/_accomplishment-banner.html
+++ b/lms/templates/certificates/_accomplishment-banner.html
@@ -1,13 +1,14 @@
+<%page expression_filter="h"/>
 <%!
 from django.utils.translation import ugettext as _
-from django.template.defaultfilters import escapejs
+from openedx.core.djangolib.js_utils import js_escaped_string
 %>
 <%namespace name='static' file='../static_content.html'/>
 <%block name="js_extra">
  <%static:js group='certificates_wv'/>
  <script type="text/javascript">
      $(document).ready(function() {
-          FaceBook.init({"facebook_app_id": '${facebook_app_id}'});
+          FaceBook.init({"facebook_app_id": '${facebook_app_id | n, js_escaped_string}'});
          $.ajaxSetup({
              headers: {
                  'X-CSRFToken': $.cookie('csrftoken')
@@ -16,15 +17,15 @@ from django.template.defaultfilters import escapejs
          });
          $(".action-linkedin-profile").click(function() {
              var data = {
-                  user_id: '${accomplishment_user_id}',
+                  user_id: '${accomplishment_user_id | n, js_escaped_string}',
                  course_id: $(this).data('course-id'),
                  enrollment_mode: $(this).data('certificate-mode'),
-                  certificate_id: '${certificate_id_number}',
+                  certificate_id: '${certificate_id_number | n, js_escaped_string}',
                  certificate_url: window.location.href,
                  social_network: 'LinkedIn'
              };
              Logger.log('edx.certificate.shared', data);
-              window.open('${linked_in_url}');
+              window.open('${linked_in_url | n, js_escaped_string}');
          });
      });

@@ -40,7 +41,7 @@ from django.template.defaultfilters import escapejs
 <div class="wrapper-banner wrapper-banner-user">
    <section class="banner banner-user">
        <div class="message message-block message-notice">
-            <h2 class="message-title hd-5 emphasized">${accomplishment_banner_opening | h}</h2>
+            <h2 class="message-title hd-5 emphasized">${accomplishment_banner_opening}</h2>
            <div class="wrapper-copy-and-actions">
                <p class="message-copy copy copy-base emphasized">${accomplishment_banner_congrats}</p>
                <div class="message-actions">
@@ -48,10 +49,10 @@ from django.template.defaultfilters import escapejs
                    % if facebook_share_enabled:
                      <button class="action action-share-facebook btn-inverse btn-small icon-only" id="action-share-facebook"
                         onclick="FaceBook.share({
-                          share_text: '${facebook_share_text | escapejs}',
-                          share_link: '${share_url}',
-                          picture_link: '${full_course_image_url}',
-                          description: '${_('Click the link to see my certificate.') | escapejs}'
+                          share_text: '${facebook_share_text | n, js_escaped_string}', ## xss-lint: disable=mako-invalid-html-filter
+                          share_link: '${share_url | n, js_escaped_string}', ## xss-lint: disable=mako-invalid-html-filter
+                          picture_link: '${full_course_image_url | n, js_escaped_string}', ## xss-lint: disable=mako-invalid-html-filter
+                          description: '${_('Click the link to see my certificate.') | n, js_escaped_string}' ## xss-lint: disable=mako-invalid-html-filter
                          });">
                          <span class="icon fa fa-facebook-official" aria-hidden="true"></span>
                          <span class="action-label">${_("Post on Facebook")}</span>
@@ -61,7 +62,8 @@ from django.template.defaultfilters import escapejs
                      <button data-tooltip="${_('Share on Twitter')}"
                        class="action action-share-twitter btn-inverse btn-small icon-only"
                        title="${_('Share on Twitter')}"
-                        onclick="popupWindow('${twitter_url}', 'tweetWindow', 640, 480); return false;">
+                        ## xss-lint: disable=mako-invalid-html-filter
+                        onclick="popupWindow('${twitter_url | n, js_escaped_string}', 'tweetWindow', 640, 480); return false;">
                          <span class="icon fa fa-twitter" aria-hidden="true"></span>
                          <span class="action-label">${_("Tweet this Accomplishment. Pop up window.")}</span>
                      </button>
--- a/lms/templates/certificates/_accomplishment-footer.html
+++ b/lms/templates/certificates/_accomplishment-footer.html
@@ -1,8 +1,9 @@
+<%page expression_filter="h"/>
 <div class="wrapper-footer">

    <footer class="footer-app" role="contentinfo" id="company-info">
        <div class="footer-app-copyright">
-            <p class="copy copy-micro">${copyright_text}</p>
+            <p class="copy copy-micro">${copyright_text | n, decode.utf8 }</p>
        </div>
        <nav class="footer-app-nav">
            <ul class="list list-legal">
--- a/lms/templates/certificates/_accomplishment-header.html
+++ b/lms/templates/certificates/_accomplishment-header.html
@@ -1,3 +1,4 @@
+<%page expression_filter="h"/>
 <%! from django.utils.translation import ugettext as _ %>

 <div class="wrapper-header">
--- a/lms/templates/certificates/_accomplishment-introduction.html
+++ b/lms/templates/certificates/_accomplishment-introduction.html
@@ -1,3 +1,4 @@
+<%page expression_filter="h"/>
 <div class="wrapper-introduction">
    <section class="introduction">
        <h2 class="introduction-copy hd-2 emphasized">
--- a/lms/templates/certificates/_accomplishment-rendering.html
+++ b/lms/templates/certificates/_accomplishment-rendering.html
@@ -1,3 +1,4 @@
+<%page expression_filter="h"/>
 <%! from django.utils.translation import ugettext as _ %>
 <%namespace name='static' file='../static_content.html'/>
 <%
@@ -24,7 +25,7 @@ course_mode_class = course_mode if course_mode else ''
            <div class="wrapper-statement-and-signatories">
                <div class="accomplishment-statement">
                    <p class="accomplishment-statement-lead">
-                        <strong class="accomplishment-recipient hd-1 emphasized">${accomplishment_copy_name | h}</strong>
+                        <strong class="accomplishment-recipient hd-1 emphasized">${accomplishment_copy_name}</strong>
                        <span class="accomplishment-summary copy copy-lead">${accomplishment_copy_description_full}</span>

                        <span class="accomplishment-course hd-1 emphasized">
@@ -86,17 +87,17 @@ course_mode_class = course_mode if course_mode else ''

    <div class="wrapper-accomplishment-metadata">
        <div class="accomplishment-metadata">
-            <h2 class="accomplishment-metadata-title hd-6">${accomplishment_copy_more_about | h}</h2>
+            <h2 class="accomplishment-metadata-title hd-6">${accomplishment_copy_more_about}</h2>

            <div class="wrapper-metadata">
                <dl class="metadata accomplishment-recipient">
-                    <dt class="label sr-only">Awarded to:</dt>
+                    <dt class="label sr-only">${_("Awarded to:")}</dt>
                    <dd class="value copy copy-meta">
                        <span class="recipient-img">
                            <img class="src" src="/static/certificates/images/demo-user-profile.png" alt="Recipient Image">
                        </span>
                        <div class="recipient-details">
-                            <h3 class="recipient-name">${accomplishment_copy_name | h}</h3>
+                            <h3 class="recipient-name">${accomplishment_copy_name}</h3>
                            <p class="recipient-username">${accomplishment_copy_username} @ ${platform_name}</p>
                        </div>
                    </dd>
--- a/lms/templates/certificates/accomplishment-base.html
+++ b/lms/templates/certificates/accomplishment-base.html
@@ -1,5 +1,6 @@
+<%page expression_filter="h"/>
 <%namespace name='static' file='/static_content.html'/>
-<%! from django.utils.translation import ugettext as _ %>
+<%! from django.utils.translation import ugettext as _%>

 <%
 # set doc language direction
@@ -9,7 +10,7 @@ course_mode_class = course_mode if course_mode else ''
 %>

 <!DOCTYPE html>
-<html class="no-js" lang="en">
+<html class="no-js" lang="${LANGUAGE_CODE}">
 <head dir="${dir_rtl}">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta charset="utf-8">
--- a/lms/templates/certificates/invalid.html
+++ b/lms/templates/certificates/invalid.html
@@ -1,3 +1,4 @@
+<%page expression_filter="h"/>
 <%inherit file="accomplishment-base.html" />
 <%! from django.utils.translation import ugettext as _ %>

@@ -11,7 +12,7 @@
                </div>
            </section>
        </main>
-        <aside role="complementary" class="content-secondary about" aria-label="About ${platform_name} Certificates">
+        <aside role="complementary" class="content-secondary about" aria-label="${_('About {platform_name} Certificates').format(platform_name=platform_name)}">
        </aside>
    </div>
 </div>
--- a/lms/templates/certificates/server-error.html
+++ b/lms/templates/certificates/server-error.html
@@ -1,3 +1,4 @@
+<%page expression_filter="h"/>
 <%! from django.utils.translation import ugettext as _ %>
 <%inherit file="../main.html" />

@@ -13,7 +14,7 @@
            ${_("To resolve the problem, your partner manager should verify that the following information is correct.")}
        </p>
        <ul>
-            <li>${_("The institution&#39;s logo.")}</li>
+            <li>${_("The institution's logo.")}</li>
            <li>${_("The institution that is linked to the course.")}</li>
            <li>${_("The course information in the Course Administration tool.")}</li>
        </ul>
--- a/lms/templates/certificates/valid.html
+++ b/lms/templates/certificates/valid.html
@@ -1,3 +1,4 @@
+<%page expression_filter="h"/>
 <%inherit file="accomplishment-base.html" />
 <%! from django.utils.translation import ugettext as _ %>

@@ -8,7 +9,7 @@

 <%include file="_accomplishment-rendering.html" />
 <div class="wrapper-about">
-    <aside role="complementary" class="about" aria-label="About edX Certificates">
+    <aside role="complementary" class="about" aria-label=${_("About edX Certificates")}>
        <%include file="_about-edx.html" />
        <%include file="_about-accomplishments.html" />
    </aside>