Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
This is another attempt at fixing
student_ans
HTML escaping issues.
This backs up one step from what was done in #1004. Instead of calling `protectHTML` on `$ans->{student_ans}` in the `cmp_error` method, that is instead called in the `cmp_parse` method in the case that the `$ans->{student_value}` is undefined. With #1004, if `student_value` was defined, and then a problem author calls `Value::Error` in a custom answer checker, it results in the `student_ans` being escaped twice because it is escaped in `cmp_parse` and then again after the custom answer checker is called. With this approach the `student_ans` is escaped when `cmp_parse` is called in either case regardless if if `student_value` is defined, and then not again later (if `student_value` is defined and thus the checker is called) This is also before the `format_matrix` method is called for array answers. This fixes an issue also caused by escaping those already HTML formatted answers. In that case the `student_ans` is completely redefined after this from the `student_formula`. With #1004, the `student_ans` is first HTML formatted in this case, and then later escaped which results in an incorrect display. This of course still fixes the issue that #1004 attempted to fix, because the `student_value` is not defined, and so the `protectHTML` now called in `cmp_parse` covers this case. I am sure that this is not the end of this issue, but is better than both before #1004 and after. I think that something along the lines of @dpvc's suggestion in #990 (comment) will be needed to fully resolve this issue. I suggest that rather than having `student_ans_text`, `student_ans_html`, `student_ans_latex`, and `preview_ans_text` as suggested there, we use `student_ans`, `preview_latex_string`, and `preview_text_string`. `student_ans` would always remain a text answer, and never be HTML formatted, `preview_ans_latex` would stay the same and be LaTeX, and `preview_text_string` would always be HTML. The naming is not the best since `preview_text_string` is actually HTML, but it is the closest to the current usage, and so would take the least work. Note that currently `preview_text_string` is not actually used for anything. Even though it is documented in `lib/AnswerHash.pm` as being what is supposed to be displayed for the preview. So this proposal just aligns that answer hash key with its intended use. Test cases: ``` DOCUMENT(); loadMacros('PGstandard.pl', 'PGML.pl'); Context('Matrix')->flags->set(requireConstantVectors => 1); Context()->variables->are(s => 'Real', t => 'Real'); $ans = Formula('s<1, 0> + t<0, 1>'); $ans_cmp = $ans->cmp( checker => sub { my ($c, $s, $self) = @_; Value::Error('Try again') unless $c == $s; return 1; } ); BEGIN_PGML Enter [`[$ans]`]: [_]{$ans_cmp} END_PGML ENDDOCUMENT(); ``` With this problem enter `<s,0>+t<0,1>`. Prior to #1004 the first vector would disappear from the displayed answer because `student_ans` was not escaped. After #1004 and still with this pull request this is displayed correctly. Also with this problem enter `s<1,0>+t<0,2>`. Prior to #1004 this displayed correctly. With #1004 it is displayed as `s*<1,0>+t*<0,2>`. With this pull request is is displayed correctly. ``` DOCUMENT(); loadMacros('PGstandard.pl', 'PGML.pl'); Context('Matrix'); $A = Matrix([ [ 1, 0 ], [ 0, -1 ] ]); BEGIN_PGML Enter the matrix [`[$A]`]: [_]*{$A} END_PGML ENDDOCUMENT(); ``` With this problem enter `S, -1, 1, 0` for the matrix entries. Prior to #1004 this displayed correctly. With #1004 this is escaped twice, and so you see the HTML code displayed. With this pull request this is again displayed correctly.
- Loading branch information