-
Bug
-
Resolution: Fixed
-
Minor
-
3.5.3, 3.6.1, 3.7
-
MOODLE_35_STABLE, MOODLE_36_STABLE, MOODLE_37_STABLE
-
MOODLE_35_STABLE, MOODLE_36_STABLE
-
MDL-64551-master -
The question bank of a course (/question/edit.php?courseid=<course_ID>) contains a tag icon whith tooltip "Manage Tags" for each question entry. Clicking on this icon opens a popup for tag management. This is implemented with JavaScript.
For cases where JavaScript is not used (users who have it disabled, screenreaders, bots, and so on), a link to the question settings is set. Its href attribute is:
/question/question.php?returnurl=%2Fquestion%2Fedit.php%3Fcourseid%3D<course_ID>&courseid=<course_ID>&id=<question_ID>
|
This is obviously wrong. (The href attribute contains the link as it should be, but with all ampersand characters ("&") replaced with "& a m p ;" (without the spaces).)
I have verified that this is a bug in the Moodle 3.5, 3.6 and 3.7 branches, and have not checked other versions.
Source of this bug is that the link around the "Manage Tags" icon is generated in a way that differs from how the other icons are generated: /question/classes/bank/tags_action_column.php uses html_writer::link for link creation (function core_question\bank\tags_action_column->print_tag_icon, called from function display_content), and the HTML writer of course correctly (HTML-)escapes URIs. But it is passed an URI which has already been HTML-escaped (via moodle_url->out). The other icons use core_question\bank\action_column_base->print_icon (in file /question/classes/bank/action_column_base.php), which does not escape{{ }}$url another time, so they work properly. (But they seem not to HTML-escape $title, which is another bug and may be spun off into another ticket.)
As I am not too experienced in this certain area, and do not see through all related classes (yet), I will not provide a patch, but can offer a suggestion: I think it may be a good idea to modify action_column_base->print_icon to escape its arguments before writing them out, and change all action column classes to always pass unescaped $url until the URI is actually printed out. In addition to this, override print_icon in tags_action_column to do its special job there. This should be a viable solution, I hope. But I do not know whether an action_link (from class core_renderer) may be better for the "Manage tags" link, as I do not have deep insights into when which link types/functions are internally used by Moodle.