Merge pull request #1002 from MITx/feature/victor/inputtypes-refactor

More inputtype refactor …

common/lib/capa/capa/inputtypes.py

@@ -21,26 +21,26 @@ Each input type takes the xml tree as 'element', the previous answer as 'value',
 graded status as'status'
 """
 
-# TODO: there is a lot of repetitive "grab these elements from xml attributes, with these defaults,
-# put them in the context" code.  Refactor so class just specifies required and optional attrs (with
-# defaults for latter), and InputTypeBase does the right thing.
+# TODO: make hints do something
+
+# TODO: make all inputtypes actually render msg
+
+# TODO: remove unused fields (e.g. 'hidden' in a few places)
+
+# TODO: add validators so that content folks get better error messages.
 
-# TODO: Quoting and unquoting is handled in a pretty ad-hoc way.  Also something that could be done
-# properly once in InputTypeBase.
 
 # Possible todo: make inline the default for textlines and other "one-line" inputs.  It probably
 # makes sense, but a bunch of problems have markup that assumes block.  Bigger TODO: figure out a
 # general css and layout strategy for capa, document it, then implement it.
 
-
-
+from collections import namedtuple
 import json
 import logging
 from lxml import etree
 import re
 import shlex  # for splitting quoted strings
 import sys
-import xml.sax.saxutils as saxutils
 
 from registry import TagRegistry
 
@@ -50,6 +50,61 @@ log = logging.getLogger('mitx.' + __name__)
 
 registry = TagRegistry()
 
+class Attribute(object):
+    """
+    Allows specifying required and optional attributes for input types.
+    """
+
+    # want to allow default to be None, but also allow required objects
+    _sentinel = object()
+
+    def __init__(self, name, default=_sentinel, transform=None, validate=None, render=True):
+        """
+        Define an attribute
+
+        name (str): then name of the attribute--should be alphanumeric (valid for an XML attribute)
+
+        default (any type): If not specified, this attribute is required.  If specified, use this as the default value
+                        if the attribute is not specified.  Note that this value will not be transformed or validated.
+
+        transform (function str -> any type): If not None, will be called to transform the parsed value into an internal
+                        representation.
+
+        validate (function str-or-return-type-of-tranform -> unit or exception): If not None, called to validate the
+                       (possibly transformed) value of the attribute.  Should raise ValueError with a helpful message if
+                       the value is invalid.
+
+        render (bool): if False, don't include this attribute in the template context.
+        """
+        self.name = name
+        self.default = default
+        self.validate = validate
+        self.transform = transform
+        self.render = render
+
+    def parse_from_xml(self, element):
+        """
+        Given an etree xml element that should have this attribute, do the obvious thing:
+          - look for it.  raise ValueError if not found and required.
+          - transform and validate.  pass through any exceptions from transform or validate.
+        """
+        val = element.get(self.name)
+        if self.default == self._sentinel and val is None:
+            raise ValueError('Missing required attribute {0}.'.format(self.name))
+
+        if val is None:
+            # not required, so return default
+            return self.default
+
+        if self.transform is not None:
+            val = self.transform(val)
+
+        if self.validate is not None:
+            self.validate(val)
+
+        return val
+
+
 class InputTypeBase(object):
     """
     Abstract base class for input types.
@@ -102,9 +157,12 @@ class InputTypeBase(object):
 
         self.status = state.get('status', 'unanswered')
 
-        # Call subclass "constructor" -- means they don't have to worry about calling
-        # super().__init__, and are isolated from changes to the input constructor interface.
         try:
+            # Pre-parse and propcess all the declared requirements.
+            self.process_requirements()
+
+            # Call subclass "constructor" -- means they don't have to worry about calling
+            # super().__init__, and are isolated from changes to the input constructor interface.
             self.setup()
         except Exception as err:
             # Something went wrong: add xml to message, but keep the traceback
@@ -112,6 +170,36 @@ class InputTypeBase(object):
             raise Exception, msg, sys.exc_info()[2]
 
 
+    @classmethod
+    def get_attributes(cls):
+        """
+        Should return a list of Attribute objects (see docstring there for details). Subclasses should override.  e.g.
+
+        return [Attribute('unicorn', True), Attribute('num_dragons', 12, transform=int), ...]
+        """
+        return []
+
+
+    def process_requirements(self):
+        """
+        Subclasses can declare lists of required and optional attributes.  This
+        function parses the input xml and pulls out those attributes.  This
+        isolates most simple input types from needing to deal with xml parsing at all.
+
+        Processes attributes, putting the results in the self.loaded_attributes dictionary.  Also creates a set
+        self.to_render, containing the names of attributes that should be included in the context by default.
+        """
+        # Use local dicts and sets so that if there are exceptions, we don't end up in a partially-initialized state.
+        loaded = {}
+        to_render = set()
+        for a in self.get_attributes():
+            loaded[a.name] = a.parse_from_xml(self.xml)
+            if a.render:
+                to_render.add(a.name)
+
+        self.loaded_attributes = loaded
+        self.to_render = to_render
+
     def setup(self):
         """
         InputTypes should override this to do any needed initialization.  It is called after the
@@ -122,14 +210,36 @@ class InputTypeBase(object):
         """
         pass
 
+
     def _get_render_context(self):
         """
-        Abstract method.  Subclasses should implement to return the dictionary
-        of keys needed to render their template.
+        Should return a dictionary of keys needed to render the template for the input type.
 
         (Separate from get_html to faciliate testing of logic separately from the rendering)
+
+        The default implementation gets the following rendering context: basic things like value, id, status, and msg,
+        as well as everything in self.loaded_attributes, and everything returned by self._extra_context().
+
+        This means that input types that only parse attributes and pass them to the template get everything they need,
+        and don't need to override this method.
         """
-        raise NotImplementedError
+        context = {
+            'id': self.id,
+            'value': self.value,
+            'status': self.status,
+            'msg': self.msg,
+            }
+        context.update((a, v) for (a, v) in self.loaded_attributes.iteritems() if a in self.to_render)
+        context.update(self._extra_context())
+        return context
+
+    def _extra_context(self):
+        """
+        Subclasses can override this to return extra context that should be passed to their templates for rendering.
+
+        This is useful when the input type requires computing new template variables from the parsed attributes.
+        """
+        return {}
 
     def get_html(self):
         """
@@ -139,7 +249,9 @@ class InputTypeBase(object):
             raise NotImplementedError("no rendering template specified for class {0}"
                                       .format(self.__class__))
 
-        html = self.system.render_template(self.template, self._get_render_context())
+        context = self._get_render_context()
+
+        html = self.system.render_template(self.template, context)
         return etree.XML(html)
 
 
@@ -153,38 +265,38 @@ class OptionInput(InputTypeBase):
     Example:
 
     <optioninput options="('Up','Down')" correct="Up"/><text>The location of the sky</text>
+
+    # TODO: allow ordering to be randomized
     """
 
     template = "optioninput.html"
     tags = ['optioninput']
 
-    def setup(self):
-        # Extract the options...
-        options = self.xml.get('options')
-        if not options:
-            raise ValueError("optioninput: Missing 'options' specification.")
-
+    @staticmethod
+    def parse_options(options):
+        """
+        Given options string, convert it into an ordered list of (option_id, option_description) tuples, where
+        id==description for now.  TODO: make it possible to specify different id and descriptions.
+        """
         # parse the set of possible options
-        oset = shlex.shlex(options[1:-1])
-        oset.quotes = "'"
-        oset.whitespace = ","
-        oset = [x[1:-1] for x  in list(oset)]
+        lexer = shlex.shlex(options[1:-1])
+        lexer.quotes = "'"
+        # Allow options to be separated by whitespace as well as commas
+        lexer.whitespace = ", "
 
-        # make ordered list with (key, value) same
-        self.osetdict = [(oset[x], oset[x]) for x in range(len(oset))]
-        # TODO: allow ordering to be randomized
+        # remove quotes
+        tokens = [x[1:-1] for x in list(lexer)]
 
-    def _get_render_context(self):
+        # make list of (option_id, option_description), with description=id
+        return [(t, t) for t in tokens]
 
-        context = {
-            'id': self.id,
-            'value': self.value,
-            'status': self.status,
-            'msg': self.msg,
-            'options': self.osetdict,
-            'inline': self.xml.get('inline',''),
-            }
-        return context
+    @classmethod
+    def get_attributes(cls):
+        """
+        Convert options to a convenient format.
+        """
+        return [Attribute('options', transform=cls.parse_options),
+                Attribute('inline', '')]
 
 registry.register(OptionInput)
 
@@ -223,53 +335,50 @@ class ChoiceGroup(InputTypeBase):
         # value.  (VS: would be nice to make this less hackish).
         if self.tag == 'choicegroup':
             self.suffix = ''
-            self.element_type = "radio"
+            self.html_input_type = "radio"
         elif self.tag == 'radiogroup':
-            self.element_type = "radio"
+            self.html_input_type = "radio"
             self.suffix = '[]'
         elif self.tag == 'checkboxgroup':
-            self.element_type = "checkbox"
+            self.html_input_type = "checkbox"
             self.suffix = '[]'
         else:
             raise Exception("ChoiceGroup: unexpected tag {0}".format(self.tag))
 
-        self.choices = extract_choices(self.xml)
+        self.choices = self.extract_choices(self.xml)
 
-    def _get_render_context(self):
-        context = {'id': self.id,
-                   'value': self.value,
-                   'status': self.status,
-                   'input_type': self.element_type,
-                   'choices': self.choices,
-                   'name_array_suffix': self.suffix}
-        return context
+    def _extra_context(self):
+        return {'input_type': self.html_input_type,
+                'choices': self.choices,
+                'name_array_suffix': self.suffix}
 
-def extract_choices(element):
-    '''
-    Extracts choices for a few input types, such as ChoiceGroup, RadioGroup and
-    CheckboxGroup.
+    @staticmethod
+    def extract_choices(element):
+        '''
+        Extracts choices for a few input types, such as ChoiceGroup, RadioGroup and
+        CheckboxGroup.
 
-    returns list of (choice_name, choice_text) tuples
+        returns list of (choice_name, choice_text) tuples
 
-    TODO: allow order of choices to be randomized, following lon-capa spec.  Use
-    "location" attribute, ie random, top, bottom.
-    '''
+        TODO: allow order of choices to be randomized, following lon-capa spec.  Use
+        "location" attribute, ie random, top, bottom.
+        '''
 
-    choices = []
+        choices = []
 
-    for choice in element:
-        if choice.tag != 'choice':
-            raise Exception(
-                "[capa.inputtypes.extract_choices] Expected a <choice> tag; got %s instead"
-                % choice.tag)
-        choice_text = ''.join([etree.tostring(x) for x in choice])
-        if choice.text is not None:
-            # TODO: fix order?
-            choice_text += choice.text
+        for choice in element:
+            if choice.tag != 'choice':
+                raise Exception(
+                    "[capa.inputtypes.extract_choices] Expected a <choice> tag; got %s instead"
+                    % choice.tag)
+            choice_text = ''.join([etree.tostring(x) for x in choice])
+            if choice.text is not None:
+                # TODO: fix order?
+                choice_text += choice.text
 
-        choices.append((choice.get("name"), choice_text))
+            choices.append((choice.get("name"), choice_text))
 
-    return choices
+        return choices
 
 
 registry.register(ChoiceGroup)
@@ -292,33 +401,23 @@ class JavascriptInput(InputTypeBase):
     template = "javascriptinput.html"
     tags = ['javascriptinput']
 
+    @classmethod
+    def get_attributes(cls):
+        """
+        Register the attributes.
+        """
+        return [Attribute('params', None),
+                Attribute('problem_state', None),
+                Attribute('display_class', None),
+                Attribute('display_file', None),]
+
+
     def setup(self):
         # Need to provide a value that JSON can parse if there is no
         # student-supplied value yet.
         if self.value == "":
             self.value = 'null'
 
-        self.params = self.xml.get('params')
-        self.problem_state = self.xml.get('problem_state')
-        self.display_class = self.xml.get('display_class')
-        self.display_file = self.xml.get('display_file')
-
-
-    def _get_render_context(self):
-        escapedict = {'"': '&quot;'}
-        value = saxutils.escape(self.value, escapedict)
-        msg   = saxutils.escape(self.msg, escapedict)
-
-        context = {'id': self.id,
-               'params': self.params,
-               'display_file': self.display_file,
-               'display_class': self.display_class,
-               'problem_state': self.problem_state,
-               'value': value,
-               'evaluation': msg,
-               }
-        return context
-
 registry.register(JavascriptInput)
 
 
@@ -326,51 +425,55 @@ registry.register(JavascriptInput)
 
 class TextLine(InputTypeBase):
     """
+    A text line input.  Can do math preview if "math"="1" is specified.
 
+    If the hidden attribute is specified, the textline is hidden and the input id is stored in a div with name equal
+    to the value of the hidden attribute.  This is used e.g. for embedding simulations turned into questions.
     """
 
     template = "textline.html"
     tags = ['textline']
 
+
+    @classmethod
+    def get_attributes(cls):
+        """
+        Register the attributes.
+        """
+        return [
+            Attribute('size', None),
+
+
+            Attribute('hidden', False),
+            Attribute('inline', False),
+
+            # Attributes below used in setup(), not rendered directly.
+            Attribute('math', None, render=False),
+            # TODO: 'dojs' flag is temporary, for backwards compatibility with 8.02x
+            Attribute('dojs', None, render=False),
+            Attribute('preprocessorClassName', None, render=False),
+            Attribute('preprocessorSrc', None, render=False),
+            ]
+
+
     def setup(self):
-        self.size = self.xml.get('size')
+        self.do_math = bool(self.loaded_attributes['math'] or
+                            self.loaded_attributes['dojs'])
 
-        # if specified, then textline is hidden and input id is stored
-        # in div with name=self.hidden.
-        self.hidden = self.xml.get('hidden', False)
-
-        self.inline = self.xml.get('inline', False)
-
-        # TODO: 'dojs' flag is temporary, for backwards compatibility with 8.02x
-        self.do_math = bool(self.xml.get('math') or self.xml.get('dojs'))
         # TODO: do math checking using ajax instead of using js, so
         # that we only have one math parser.
         self.preprocessor = None
         if self.do_math:
             # Preprocessor to insert between raw input and Mathjax
-            self.preprocessor = {'class_name': self.xml.get('preprocessorClassName',''),
-                            'script_src': self.xml.get('preprocessorSrc','')}
-            if '' in self.preprocessor.values():
+            self.preprocessor = {'class_name': self.loaded_attributes['preprocessorClassName'],
+                                 'script_src': self.loaded_attributes['preprocessorSrc']}
+            if None in self.preprocessor.values():
                 self.preprocessor = None
 
 
-
-    def _get_render_context(self):
-        # Escape answers with quotes, so they don't crash the system!
-        escapedict = {'"': '&quot;'}
-        value = saxutils.escape(self.value, escapedict)
-
-        context = {'id': self.id,
-                   'value': value,
-                   'status': self.status,
-                   'size': self.size,
-                   'msg': self.msg,
-                   'hidden': self.hidden,
-                   'inline': self.inline,
-                   'do_math': self.do_math,
-                   'preprocessor': self.preprocessor,
-               }
-        return context
+    def _extra_context(self):
+        return {'do_math': self.do_math,
+                'preprocessor': self.preprocessor,}
 
 registry.register(TextLine)
 
@@ -388,13 +491,26 @@ class FileSubmission(InputTypeBase):
     submitted_msg = ("Your file(s) have been submitted; as soon as your submission is"
                      " graded, this message will be replaced with the grader's feedback.")
 
-    def setup(self):
-        escapedict = {'"': '&quot;'}
-        self.allowed_files  = json.dumps(self.xml.get('allowed_files', '').split())
-        self.allowed_files  = saxutils.escape(self.allowed_files, escapedict)
-        self.required_files = json.dumps(self.xml.get('required_files', '').split())
-        self.required_files = saxutils.escape(self.required_files, escapedict)
+    @staticmethod
+    def parse_files(files):
+        """
+        Given a string like 'a.py b.py c.out', split on whitespace and return as a json list.
+        """
+        return json.dumps(files.split())
 
+    @classmethod
+    def get_attributes(cls):
+        """
+        Convert the list of allowed files to a convenient format.
+        """
+        return [Attribute('allowed_files', '[]', transform=cls.parse_files),
+                Attribute('required_files', '[]', transform=cls.parse_files),]
+
+    def setup(self):
+        """
+        Do some magic to handle queueing status (render as "queued" instead of "incomplete"),
+        pull queue_len from the msg field.  (TODO: get rid of the queue_len hack).
+        """
         # Check if problem has been queued
         self.queue_len = 0
         # Flag indicating that the problem has been queued, 'msg' is length of queue
@@ -403,15 +519,8 @@ class FileSubmission(InputTypeBase):
             self.queue_len = self.msg
             self.msg = FileSubmission.submitted_msg
 
-    def _get_render_context(self):
-
-        context = {'id': self.id,
-                   'status': self.status,
-                   'msg': self.msg,
-                   'value': self.value,
-                   'queue_len': self.queue_len,
-                   'allowed_files': self.allowed_files,
-                   'required_files': self.required_files,}
+    def _extra_context(self):
+        return {'queue_len': self.queue_len,}
         return context
 
 registry.register(FileSubmission)
@@ -431,13 +540,30 @@ class CodeInput(InputTypeBase):
                               # non-codemirror editor.
             ]
 
+    # pulled out for testing
+    submitted_msg = ("Submitted.  As soon as your submission is"
+                     " graded, this message will be replaced with the grader's feedback.")
+
+    @classmethod
+    def get_attributes(cls):
+        """
+        Convert options to a convenient format.
+        """
+        return [Attribute('rows', '30'),
+                Attribute('cols', '80'),
+                Attribute('hidden', ''),
+
+                # For CodeMirror
+                Attribute('mode', 'python'),
+                Attribute('linenumbers', 'true'),
+                # Template expects tabsize to be an int it can do math with
+                Attribute('tabsize', 4, transform=int),
+                ]
 
     def setup(self):
-        self.rows = self.xml.get('rows') or '30'
-        self.cols = self.xml.get('cols') or '80'
-        # if specified, then textline is hidden and id is stored in div of name given by hidden
-        self.hidden = self.xml.get('hidden', '')
-
+        """
+        Implement special logic: handle queueing state, and default input.
+        """
         # if no student input yet, then use the default input given by the problem
         if not self.value:
             self.value = self.xml.text
@@ -448,28 +574,11 @@ class CodeInput(InputTypeBase):
         if self.status == 'incomplete':
             self.status = 'queued'
             self.queue_len = self.msg
-            self.msg = 'Submitted to grader.'
+            self.msg = self.submitted_msg
 
-        # For CodeMirror
-        self.mode = self.xml.get('mode', 'python')
-        self.linenumbers = self.xml.get('linenumbers', 'true')
-        self.tabsize = int(self.xml.get('tabsize', '4'))
-
-    def _get_render_context(self):
-
-        context = {'id': self.id,
-                   'value': self.value,
-                   'status': self.status,
-                   'msg': self.msg,
-                   'mode': self.mode,
-                   'linenumbers': self.linenumbers,
-                   'rows': self.rows,
-                   'cols': self.cols,
-                   'hidden': self.hidden,
-                   'tabsize': self.tabsize,
-                   'queue_len': self.queue_len,
-               }
-        return context
+    def _extra_context(self):
+        """Defined queue_len, add it """
+        return {'queue_len': self.queue_len,}
 
 registry.register(CodeInput)
 
@@ -482,26 +591,19 @@ class Schematic(InputTypeBase):
     template = "schematicinput.html"
     tags = ['schematic']
 
-    def setup(self):
-        self.height = self.xml.get('height')
-        self.width = self.xml.get('width')
-        self.parts = self.xml.get('parts')
-        self.analyses = self.xml.get('analyses')
-        self.initial_value = self.xml.get('initial_value')
-        self.submit_analyses = self.xml.get('submit_analyses')
+    @classmethod
+    def get_attributes(cls):
+        """
+        Convert options to a convenient format.
+        """
+        return [
+            Attribute('height', None),
+            Attribute('width', None),
+            Attribute('parts', None),
+            Attribute('analyses', None),
+            Attribute('initial_value', None),
+            Attribute('submit_analyses', None),]
 
-
-    def _get_render_context(self):
-
-        context = {'id': self.id,
-                   'value': self.value,
-                   'initial_value': self.initial_value,
-                   'status': self.status,
-                   'width': self.width,
-                   'height': self.height,
-                   'parts': self.parts,
-                   'analyses': self.analyses,
-                   'submit_analyses': self.submit_analyses,}
         return context
 
 registry.register(Schematic)
@@ -522,12 +624,20 @@ class ImageInput(InputTypeBase):
     template = "imageinput.html"
     tags = ['imageinput']
 
-    def setup(self):
-        self.src = self.xml.get('src')
-        self.height = self.xml.get('height')
-        self.width = self.xml.get('width')
+    @classmethod
+    def get_attributes(cls):
+        """
+        Note: src, height, and width are all required.
+        """
+        return [Attribute('src'),
+                Attribute('height'),
+                Attribute('width'),]
 
-        # if value is of the form [x,y] then parse it and send along coordinates of previous answer
+
+    def setup(self):
+        """
+        if value is of the form [x,y] then parse it and send along coordinates of previous answer
+        """
         m = re.match('\[([0-9]+),([0-9]+)]', self.value.strip().replace(' ', ''))
         if m:
             # Note: we subtract 15 to compensate for the size of the dot on the screen.
@@ -537,19 +647,10 @@ class ImageInput(InputTypeBase):
             (self.gx, self.gy) = (0, 0)
 
 
-    def _get_render_context(self):
+    def _extra_context(self):
 
-        context = {'id': self.id,
-                   'value': self.value,
-                   'height': self.height,
-                   'width': self.width,
-                   'src': self.src,
-                   'gx': self.gx,
-                   'gy': self.gy,
-                   'status': self.status,
-                   'msg': self.msg,
-               }
-        return context
+        return {'gx': self.gx,
+                'gy': self.gy}
 
 registry.register(ImageInput)
 
@@ -565,30 +666,18 @@ class Crystallography(InputTypeBase):
     template = "crystallography.html"
     tags = ['crystallography']
 
+    @classmethod
+    def get_attributes(cls):
+        """
+        Note: height, width are required.
+        """
+        return [Attribute('size', None),
+                Attribute('height'),
+                Attribute('width'),
 
-    def setup(self):
-        self.height = self.xml.get('height')
-        self.width = self.xml.get('width')
-        self.size = self.xml.get('size')
-
-        # if specified, then textline is hidden and id is stored in div of name given by hidden
-        self.hidden = self.xml.get('hidden', '')
-
-        # Escape answers with quotes, so they don't crash the system!
-        escapedict = {'"': '&quot;'}
-        self.value = saxutils.escape(self.value, escapedict)
-
-    def _get_render_context(self):
-        context = {'id': self.id,
-                   'value': self.value,
-                   'status': self.status,
-                   'size': self.size,
-                   'msg': self.msg,
-                   'hidden': self.hidden,
-                   'width': self.width,
-                   'height': self.height,
-               }
-        return context
+                # can probably be removed (textline should prob be always-hidden)
+                Attribute('hidden', ''),
+                ]
 
 registry.register(Crystallography)
 
@@ -603,29 +692,16 @@ class VseprInput(InputTypeBase):
     template = 'vsepr_input.html'
     tags = ['vsepr_input']
 
-    def setup(self):
-        self.height = self.xml.get('height')
-        self.width = self.xml.get('width')
-
-        # Escape answers with quotes, so they don't crash the system!
-        escapedict = {'"': '&quot;'}
-        self.value = saxutils.escape(self.value, escapedict)
-
-        self.molecules = self.xml.get('molecules')
-        self.geometries = self.xml.get('geometries')
-
-    def _get_render_context(self):
-
-        context = {'id': self.id,
-                   'value': self.value,
-                   'status': self.status,
-                   'msg': self.msg,
-                   'width': self.width,
-                   'height': self.height,
-                   'molecules': self.molecules,
-                   'geometries': self.geometries,
-               }
-        return context
+    @classmethod
+    def get_attributes(cls):
+        """
+        Note: height, width are required.
+        """
+        return [Attribute('height'),
+                Attribute('width'),
+                Attribute('molecules'),
+                Attribute('geometries'),
+                ]
 
 registry.register(VseprInput)
 
@@ -646,17 +722,17 @@ class ChemicalEquationInput(InputTypeBase):
     template = "chemicalequationinput.html"
     tags = ['chemicalequationinput']
 
-    def setup(self):
-        self.size = self.xml.get('size', '20')
+    @classmethod
+    def get_attributes(cls):
+        """
+        Can set size of text field.
+        """
+        return [Attribute('size', '20'),]
 
-    def _get_render_context(self):
-        context = {
-            'id': self.id,
-            'value': self.value,
-            'status': self.status,
-            'size': self.size,
-            'previewer': '/static/js/capa/chemical_equation_preview.js',
-            }
-        return context
+    def _extra_context(self):
+        """
+        TODO (vshnayder): Get rid of this once we have a standard way of requiring js to be loaded.
+        """
+        return {'previewer': '/static/js/capa/chemical_equation_preview.js',}
 
 registry.register(ChemicalEquationInput)

common/lib/capa/capa/templates/crystallography.html

@@ -19,7 +19,7 @@
       <div style="display:none;" name="${hidden}" inputid="input_${id}" />
     % endif
 
-  <input type="text" name="input_${id}" id="input_${id}" value="${value}"
+  <input type="text" name="input_${id}" id="input_${id}" value="${value|h}"
    % if size:
    size="${size}"
    % endif

common/lib/capa/capa/templates/filesubmission.html

@@ -12,7 +12,7 @@
     % endif
     <p class="debug">${status}</p>
 
-    <input type="file" name="input_${id}" id="input_${id}" value="${value}" multiple="multiple" data-required_files="${required_files}" data-allowed_files="${allowed_files}"/>
+    <input type="file" name="input_${id}" id="input_${id}" value="${value}" multiple="multiple" data-required_files="${required_files|h}" data-allowed_files="${allowed_files|h}"/>
   </div>
     <div class="message">${msg|n}</div>
 </section>

common/lib/capa/capa/templates/javascriptinput.html

@@ -2,7 +2,7 @@
   <input type="hidden" name="input_${id}" id="input_${id}" class="javascriptinput_input"/>
     <div class="javascriptinput_data" data-display_class="${display_class}" 
         data-problem_state="${problem_state}" data-params="${params}" 
-        data-submission="${value}" data-evaluation="${evaluation}">
+        data-submission="${value|h}" data-evaluation="${msg|h}">
     </div>
     <div class="script_placeholder" data-src="/static/js/${display_file}"></div>
     <div class="javascriptinput_container"></div>

common/lib/capa/capa/templates/textline.html

@@ -20,7 +20,7 @@
       <div style="display:none;" name="${hidden}" inputid="input_${id}" />
     % endif
 
-  <input type="text" name="input_${id}" id="input_${id}" value="${value}"
+  <input type="text" name="input_${id}" id="input_${id}" value="${value|h}"
         % if do_math:
             class="math"
         % endif

common/lib/capa/capa/templates/vsepr_input.html

@@ -21,7 +21,7 @@
     <div class="incorrect" id="status_${id}">
     % endif
 
-  <input type="text" name="input_${id}" id="input_${id}" value="${value}"
+  <input type="text" name="input_${id}" id="input_${id}" value="${value|h}"
       style="display:none;"
    />
 

common/lib/capa/capa/tests/test_inputtypes.py

@@ -2,9 +2,18 @@
 Tests of input types.
 
 TODO:
+- refactor: so much repetive code (have factory methods that build xml elements directly, etc)
+
+- test error cases
+
+- check rendering -- e.g. msg should appear in the rendered output.  If possible, test that
+  templates are escaping things properly.
+
+  
 - test unicode in values, parameters, etc.
 - test various html escapes
 - test funny xml chars -- should never get xml parse error if things are escaped properly.
+
 """
 
 from lxml import etree
@@ -46,6 +55,19 @@ class OptionInputTest(unittest.TestCase):
 
         self.assertEqual(context, expected)
 
+    def test_option_parsing(self):
+        f = inputtypes.OptionInput.parse_options
+        def check(input, options):
+            """Take list of options, confirm that output is in the silly doubled format"""
+            expected = [(o, o) for o in options]
+            self.assertEqual(f(input), expected)
+
+        check("('a','b')", ['a', 'b'])
+        check("('a', 'b')", ['a', 'b'])
+        check("('a b','b')", ['a b', 'b'])
+        check("('My \"quoted\"place','b')", ['My \"quoted\"place', 'b'])
+
+
 class ChoiceGroupTest(unittest.TestCase):
     '''
     Test choice groups, radio groups, and checkbox groups
@@ -73,6 +95,7 @@ class ChoiceGroupTest(unittest.TestCase):
         expected = {'id': 'sky_input',
                     'value': 'foil3',
                     'status': 'answered',
+                    'msg': '',
                     'input_type': expected_input_type,
                     'choices': [('foil1', '<text>This is foil One.</text>'),
                                 ('foil2', '<text>This is foil Two.</text>'),
@@ -119,12 +142,13 @@ class JavascriptInputTest(unittest.TestCase):
         context = the_input._get_render_context()
 
         expected = {'id': 'prob_1_2',
+                    'status': 'unanswered',
+                    'msg': '',
+                    'value': '3',
                     'params': params,
                     'display_file': display_file,
                     'display_class': display_class,
-                    'problem_state': problem_state,
-                    'value': '3',
-                    'evaluation': '',}
+                    'problem_state': problem_state,}
 
         self.assertEqual(context, expected)
 
@@ -204,9 +228,6 @@ class FileSubmissionTest(unittest.TestCase):
 
         element = etree.fromstring(xml_str)
 
-        escapedict = {'"': '&quot;'}
-        esc = lambda s: saxutils.escape(s, escapedict)
-
         state = {'value': 'BumbleBee.py',
                  'status': 'incomplete',
                  'feedback' : {'message': '3'}, }
@@ -220,8 +241,8 @@ class FileSubmissionTest(unittest.TestCase):
                    'msg': input_class.submitted_msg,
                    'value': 'BumbleBee.py',
                    'queue_len': '3',
-                   'allowed_files': esc('["runme.py", "nooooo.rb", "ohai.java"]'),
-                   'required_files': esc('["cookies.py"]')}
+                   'allowed_files': '["runme.py", "nooooo.rb", "ohai.java"]',
+                   'required_files': '["cookies.py"]'}
 
         self.assertEqual(context, expected)
 
@@ -255,14 +276,15 @@ class CodeInputTest(unittest.TestCase):
                  'status': 'incomplete',
                  'feedback' : {'message': '3'}, }
 
-        the_input = lookup_tag('codeinput')(test_system, element, state)
+        input_class = lookup_tag('codeinput')
+        the_input = input_class(test_system, element, state)
 
         context = the_input._get_render_context()
 
         expected = {'id': 'prob_1_2',
                     'value': 'print "good evening"',
                    'status': 'queued',
-                   'msg': 'Submitted to grader.',
+                   'msg': input_class.submitted_msg,
                    'mode': mode,
                    'linenumbers': linenumbers,
                    'rows': rows,
@@ -311,8 +333,9 @@ class SchematicTest(unittest.TestCase):
 
         expected = {'id': 'prob_1_2',
                     'value': value,
-                    'initial_value': initial_value,
                     'status': 'unsubmitted',
+                    'msg': '',
+                    'initial_value': initial_value,
                     'width': width,
                     'height': height,
                     'parts': parts,
@@ -476,6 +499,7 @@ class ChemicalEquationTest(unittest.TestCase):
         expected = {'id': 'prob_1_2',
                     'value': 'H2OYeah',
                     'status': 'unanswered',
+                    'msg': '',
                     'size': size,
                     'previewer': '/static/js/capa/chemical_equation_preview.js',
                     }