diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 066d83ed3e..80514cf8d4 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -125,7 +125,7 @@ def add_histogram(get_html, module, user): mstart = getattr(module.descriptor,'start') if mstart is not None: is_released = "Yes!" if (now > mstart) else "Not yet" - + staff_context = {'definition': module.definition.get('data'), 'metadata': json.dumps(module.metadata, indent=4), 'location': module.location, @@ -133,6 +133,7 @@ def add_histogram(get_html, module, user): 'source_file' : source_file, 'source_url': '%s/%s/tree/master/%s' % (giturl,data_dir,source_file), 'category': str(module.__class__.__name__), + # Template uses element_id in js function names, so can't allow dashes 'element_id': module.location.html_id().replace('-','_'), 'edit_link': edit_link, 'user': user, diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index c3bbc1e508..9d51ab7207 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -29,6 +29,9 @@ INVALID_CHARS = re.compile(r"[^\w.-]") # Names are allowed to have colons. INVALID_CHARS_NAME = re.compile(r"[^\w.:-]") +# html ids can contain word chars and dashes +INVALID_HTML_CHARS = re.compile(r"[^\w-]") + _LocationBase = namedtuple('LocationBase', 'tag org course category name revision') @@ -44,12 +47,35 @@ class Location(_LocationBase): ''' __slots__ = () + @staticmethod + def _clean(value, invalid): + """ + invalid should be a compiled regexp of chars to replace with '_' + """ + return re.sub('_+', '_', invalid.sub('_', value)) + + @staticmethod def clean(value): """ Return value, made into a form legal for locations """ - return re.sub('_+', '_', INVALID_CHARS.sub('_', value)) + return Location._clean(value, INVALID_CHARS) + + @staticmethod + def clean_for_url_name(value): + """ + Convert value into a format valid for location names (allows colons). + """ + return Location._clean(value, INVALID_CHARS_NAME) + + @staticmethod + def clean_for_html(value): + """ + Convert a string into a form that's safe for use in html ids, classes, urls, etc. + Replaces all INVALID_HTML_CHARS with '_', collapses multiple '_' chars + """ + return Location._clean(value, INVALID_HTML_CHARS) @staticmethod def is_valid(value): @@ -183,9 +209,9 @@ class Location(_LocationBase): Return a string with a version of the location that is safe for use in html id attributes """ - # TODO: is ':' ok in html ids? - return "-".join(str(v) for v in self.list() - if v is not None).replace('.', '_') + s = "-".join(str(v) for v in self.list() + if v is not None) + return Location.clean_for_html(s) def dict(self): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py index 529b1f88eb..afe5e47d10 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py @@ -114,12 +114,44 @@ def test_equality(): Location('tag', 'org', 'course', 'category', 'name') ) +# All the cleaning functions should do the same thing with these +general_pairs = [ ('',''), + (' ', '_'), + ('abc,', 'abc_'), + ('ab fg!@//\\aj', 'ab_fg_aj'), + (u"ab\xA9", "ab_"), # no unicode allowed for now + ] + def test_clean(): - pairs = [ ('',''), - (' ', '_'), - ('abc,', 'abc_'), - ('ab fg!@//\\aj', 'ab_fg_aj'), - (u"ab\xA9", "ab_"), # no unicode allowed for now - ] + pairs = general_pairs + [ + ('a:b', 'a_b'), # no colons in non-name components + ('a-b', 'a-b'), # dashes ok + ('a.b', 'a.b'), # dot ok + ] for input, output in pairs: assert_equals(Location.clean(input), output) + + +def test_clean_for_url_name(): + pairs = general_pairs + [ + ('a:b', 'a:b'), # colons ok in names + ('a-b', 'a-b'), # dashes ok in names + ('a.b', 'a.b'), # dot ok in names + ] + for input, output in pairs: + assert_equals(Location.clean_for_url_name(input), output) + + +def test_clean_for_html(): + pairs = general_pairs + [ + ("a:b", "a_b"), # no colons for html use + ("a-b", "a-b"), # dashes ok (though need to be replaced in various use locations. ugh.) + ('a.b', 'a_b'), # no dots. + ] + for input, output in pairs: + assert_equals(Location.clean_for_html(input), output) + + +def test_html_id(): + loc = Location("tag://org/course/cat/name:more_name@rev") + assert_equals(loc.html_id(), "tag-org-course-cat-name_more_name-rev") diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 547df36471..a5b3460f17 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -28,7 +28,7 @@ def is_pointer_tag(xml_obj): No children, one attribute named url_name. Special case for course roots: the pointer is - + xml_obj: an etree Element