Merge pull request #10962 from edx/robrap/TNL-3425
Deprecate escaping in display_name_with_default
This commit is contained in:
@@ -120,7 +120,7 @@ class ChooseModeView(View):
|
||||
"course_modes_choose_url": reverse("course_modes_choose", kwargs={'course_id': course_key.to_deprecated_string()}),
|
||||
"modes": modes,
|
||||
"has_credit_upsell": has_credit_upsell,
|
||||
"course_name": course.display_name_with_default,
|
||||
"course_name": course.display_name_with_default_escaped,
|
||||
"course_org": course.display_org_with_default,
|
||||
"course_num": course.display_number_with_default,
|
||||
"chosen_price": chosen_price,
|
||||
|
||||
@@ -150,7 +150,7 @@ class AnnotatableModule(AnnotatableFields, XModule):
|
||||
def get_html(self):
|
||||
""" Renders parameters to template. """
|
||||
context = {
|
||||
'display_name': self.display_name_with_default,
|
||||
'display_name': self.display_name_with_default_escaped,
|
||||
'element_id': self.element_id,
|
||||
'instructions_html': self.instructions,
|
||||
'content_html': self._render_content()
|
||||
|
||||
@@ -658,7 +658,7 @@ class CapaMixin(CapaFields):
|
||||
check_button_checking = False
|
||||
|
||||
content = {
|
||||
'name': self.display_name_with_default,
|
||||
'name': self.display_name_with_default_escaped,
|
||||
'html': html,
|
||||
'weight': self.weight,
|
||||
}
|
||||
|
||||
@@ -57,15 +57,51 @@ def display_name_with_default(course):
|
||||
like to just pass course.display_name and course.url_name as arguments to
|
||||
this function, we can't do so without breaking those tests.
|
||||
|
||||
Note: This method no longer escapes as it once did, so the caller must
|
||||
ensure it is properly escaped where necessary.
|
||||
|
||||
Arguments:
|
||||
course (CourseDescriptor|CourseOverview): descriptor or overview of
|
||||
said course.
|
||||
"""
|
||||
# TODO: Consider changing this to use something like xml.sax.saxutils.escape
|
||||
return (
|
||||
course.display_name if course.display_name is not None
|
||||
else course.url_name.replace('_', ' ')
|
||||
).replace('<', '<').replace('>', '>')
|
||||
)
|
||||
|
||||
|
||||
def display_name_with_default_escaped(course):
|
||||
"""
|
||||
DEPRECATED: use display_name_with_default
|
||||
|
||||
Calculates the display name for a course with some HTML escaping.
|
||||
This follows the same logic as display_name_with_default, with
|
||||
the addition of the escaping.
|
||||
|
||||
Here is an example of how to move away from this method in Mako html:
|
||||
Before:
|
||||
<span class="course-name">${course.display_name_with_default_escaped}</span>
|
||||
|
||||
After:
|
||||
<span class="course-name">${course.display_name_with_default | h}</span>
|
||||
If the context is Javascript in Mako, you'll need to follow other best practices.
|
||||
|
||||
Note: Switch to display_name_with_default, and ensure the caller
|
||||
properly escapes where necessary.
|
||||
|
||||
Note: This newly introduced method should not be used. It was only
|
||||
introduced to enable a quick search/replace and the ability to slowly
|
||||
migrate and test switching to display_name_with_default, which is no
|
||||
longer escaped.
|
||||
|
||||
Arguments:
|
||||
course (CourseDescriptor|CourseOverview): descriptor or overview of
|
||||
said course.
|
||||
"""
|
||||
# This escaping is incomplete. However, rather than switching this to use
|
||||
# markupsafe.escape() and fixing issues, better to put that energy toward
|
||||
# migrating away from this method altogether.
|
||||
return course.display_name_with_default.replace('<', '<').replace('>', '>')
|
||||
|
||||
|
||||
def number_for_course_location(location):
|
||||
|
||||
@@ -127,7 +127,7 @@ class ImageAnnotationModule(AnnotatableFields, XModule):
|
||||
def student_view(self, context):
|
||||
""" Renders parameters to template. """
|
||||
context = {
|
||||
'display_name': self.display_name_with_default,
|
||||
'display_name': self.display_name_with_default_escaped,
|
||||
'instructions_html': self.instructions,
|
||||
'token': retrieve_token(self.user_email, self.annotation_token_secret),
|
||||
'tag': self.instructor_tags,
|
||||
|
||||
@@ -224,7 +224,7 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule):
|
||||
'path': " > ".join(display_names + [child.display_name or '']),
|
||||
}
|
||||
if childinfo['title'] == '':
|
||||
childinfo['title'] = child.display_name_with_default
|
||||
childinfo['title'] = child.display_name_with_default_escaped
|
||||
contents.append(childinfo)
|
||||
|
||||
params = {
|
||||
|
||||
@@ -11,6 +11,7 @@ from xmodule.course_metadata_utils import (
|
||||
clean_course_key,
|
||||
url_name_for_course_location,
|
||||
display_name_with_default,
|
||||
display_name_with_default_escaped,
|
||||
number_for_course_location,
|
||||
has_course_started,
|
||||
has_course_ended,
|
||||
@@ -133,12 +134,18 @@ class CourseMetadataUtilsTestCase(TestCase):
|
||||
TestScenario((self.demo_course.location,), self.demo_course.location.name),
|
||||
TestScenario((self.html_course.location,), self.html_course.location.name),
|
||||
]),
|
||||
FunctionTest(display_name_with_default, [
|
||||
FunctionTest(display_name_with_default_escaped, [
|
||||
# Test course with no display name.
|
||||
TestScenario((self.demo_course,), "Empty"),
|
||||
# Test course with a display name that contains characters that need escaping.
|
||||
TestScenario((self.html_course,), "Intro to <html>"),
|
||||
]),
|
||||
FunctionTest(display_name_with_default, [
|
||||
# Test course with no display name.
|
||||
TestScenario((self.demo_course,), "Empty"),
|
||||
# Test course with a display name that contains characters that need escaping.
|
||||
TestScenario((self.html_course,), "Intro to <html>"),
|
||||
]),
|
||||
FunctionTest(number_for_course_location, [
|
||||
TestScenario((self.demo_course.location,), "DemoX.1"),
|
||||
TestScenario((self.html_course.location,), "CS-203"),
|
||||
|
||||
@@ -121,7 +121,7 @@ class TextAnnotationModule(AnnotatableFields, XModule):
|
||||
""" Renders parameters to template. """
|
||||
context = {
|
||||
'course_key': self.runtime.course_id,
|
||||
'display_name': self.display_name_with_default,
|
||||
'display_name': self.display_name_with_default_escaped,
|
||||
'tag': self.instructor_tags,
|
||||
'source': self.source,
|
||||
'instructions_html': self.instructions,
|
||||
|
||||
@@ -334,7 +334,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
|
||||
'cdn_eval': cdn_eval,
|
||||
'cdn_exp_group': cdn_exp_group,
|
||||
'id': self.location.html_id(),
|
||||
'display_name': self.display_name_with_default,
|
||||
'display_name': self.display_name_with_default_escaped,
|
||||
'handout': self.handout,
|
||||
'download_video_link': download_video_link,
|
||||
'track': track_url,
|
||||
|
||||
@@ -128,7 +128,7 @@ class VideoAnnotationModule(AnnotatableFields, XModule):
|
||||
|
||||
context = {
|
||||
'course_key': self.runtime.course_id,
|
||||
'display_name': self.display_name_with_default,
|
||||
'display_name': self.display_name_with_default_escaped,
|
||||
'instructions_html': self.instructions,
|
||||
'sourceUrl': self.sourceurl,
|
||||
'typeSource': extension,
|
||||
|
||||
@@ -348,6 +348,21 @@ class XModuleMixin(XModuleFields, XBlock):
|
||||
"""
|
||||
return course_metadata_utils.display_name_with_default(self)
|
||||
|
||||
@property
|
||||
def display_name_with_default_escaped(self):
|
||||
"""
|
||||
DEPRECATED: use display_name_with_default
|
||||
|
||||
Return an html escaped display name for the module: use display_name if
|
||||
defined in metadata, otherwise convert the url name.
|
||||
|
||||
Note: This newly introduced method should not be used. It was only
|
||||
introduced to enable a quick search/replace and the ability to slowly
|
||||
migrate and test switching to display_name_with_default, which is no
|
||||
longer escaped.
|
||||
"""
|
||||
return course_metadata_utils.display_name_with_default_escaped(self)
|
||||
|
||||
@property
|
||||
def xblock_kvs(self):
|
||||
"""
|
||||
@@ -424,7 +439,7 @@ class XModuleMixin(XModuleFields, XBlock):
|
||||
if self.has_children:
|
||||
return sum((child.get_content_titles() for child in self.get_children()), [])
|
||||
else:
|
||||
return [self.display_name_with_default]
|
||||
return [self.display_name_with_default_escaped]
|
||||
|
||||
def get_children(self, usage_id_filter=None, usage_key_filter=None): # pylint: disable=arguments-differ
|
||||
"""Returns a list of XBlock instances for the children of
|
||||
|
||||
@@ -4,7 +4,7 @@ ${_("Dear student,")}
|
||||
|
||||
${_("You have been invited to join {course_name} at {site_name} by a "
|
||||
"member of the course staff.").format(
|
||||
course_name=course.display_name_with_default,
|
||||
course_name=course.display_name_with_default_escaped,
|
||||
site_name=site_name
|
||||
)}
|
||||
|
||||
@@ -16,7 +16,7 @@ ${_("To finish your registration, please visit {registration_url} and fill "
|
||||
% if auto_enroll:
|
||||
${_("Once you have registered and activated your account, you will see "
|
||||
"{course_name} listed on your dashboard.").format(
|
||||
course_name=course.display_name_with_default
|
||||
course_name=course.display_name_with_default_escaped
|
||||
)}
|
||||
% else:
|
||||
${_("Once you have registered and activated your account, visit {course_about_url} "
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<%! from django.utils.translation import ugettext as _ %>
|
||||
|
||||
${_("You have been invited to register for {course_name}").format(
|
||||
course_name=course.display_name_with_default
|
||||
course_name=course.display_name_with_default_escaped
|
||||
)}
|
||||
@@ -5,7 +5,7 @@ ${_("Dear {full_name}").format(full_name=full_name)}
|
||||
${_("You have been enrolled in {course_name} at {site_name} by a member "
|
||||
"of the course staff. The course should now appear on your {site_name} "
|
||||
"dashboard.").format(
|
||||
course_name=course.display_name_with_default,
|
||||
course_name=course.display_name_with_default_escaped,
|
||||
site_name=site_name
|
||||
)}
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<%! from django.utils.translation import ugettext as _ %>
|
||||
|
||||
${_("You have been enrolled in {course_name}").format(
|
||||
course_name=course.display_name_with_default
|
||||
course_name=course.display_name_with_default_escaped
|
||||
)}
|
||||
@@ -4,7 +4,7 @@ ${_("Dear Student,")}
|
||||
|
||||
${_("You have been un-enrolled from course {course_name} by a member "
|
||||
"of the course staff. Please disregard the invitation "
|
||||
"previously sent.").format(course_name=course.display_name_with_default)}
|
||||
"previously sent.").format(course_name=course.display_name_with_default_escaped)}
|
||||
|
||||
----
|
||||
${_("This email was automatically sent from {site_name} "
|
||||
|
||||
@@ -5,13 +5,13 @@ ${_("Dear {full_name}").format(full_name=full_name)}
|
||||
${_("You have been un-enrolled in {course_name} at {site_name} by a member "
|
||||
"of the course staff. The course will no longer appear on your "
|
||||
"{site_name} dashboard.").format(
|
||||
course_name=course.display_name_with_default, site_name=site_name
|
||||
course_name=course.display_name_with_default_escaped, site_name=site_name
|
||||
)}
|
||||
|
||||
${_("Your other courses have not been affected.")}
|
||||
|
||||
----
|
||||
${_("This email was automatically sent from {site_name} to "
|
||||
"{full_name}").format(
|
||||
full_name=full_name, site_name=site_name
|
||||
)}
|
||||
"{full_name}").format(
|
||||
full_name=full_name, site_name=site_name
|
||||
)}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<%! from django.utils.translation import ugettext as _ %>
|
||||
|
||||
${_("You have been un-enrolled from {course_name}").format(
|
||||
course_name=course.display_name_with_default
|
||||
course_name=course.display_name_with_default_escaped
|
||||
)}
|
||||
Reference in New Issue
Block a user