maths / moodle-qtype_stack

Stack question type for Moodle
GNU General Public License v3.0
142 stars 149 forks source link

Question Blocks and &gt ; #433

Closed jmbragg closed 4 months ago

jmbragg commented 5 years ago

This looks similar to #174 . I'm trying to implement one of the Question Blocks into one of my questions to show some different work depending on the variables in the problem. Its a really cool feature but my issues are coming in from the Atto editor replacing all my > signs in my if tests with &gt ; and thus breaking the CAS evaluation. I can edit it in the plain text editor and save it successfully. But if I ever revisit the page in Atto it will "fix" them all back again and break the question... Is there a way to implement some ease of use fix for this? I'm going to continue along with my plans and just be absurdly careful... but just thought I'd make the suggestion since this seems like a very likely problem.

As I was typing this it occurred to me that I could put the predicate test in the question variables and then just call that variable that would be equal to true or false directly in the question. That seems less than ideal. But if there's not easy way to fix this then a good note about this issue in the documentation will help. I'll volunteer some newbie text if you think that's the best way to proceed.

aharjula commented 5 years ago

Well, those WYSIWYG editors are a real problem as they do their thing on the client side and we have very little control over them there. We might consider adding a flag to the question that would signal that this question contains content that does not work with WYSIWYG editors and the editor rendering logic would then make sure that that setting about editors is always correct when opening the question. It would make it simpler to control the editor at question level, so that people do not need to go to their user preferences.

From the block systems side there exists no general solution as the attributes may contain both entities and those raw signs depending on where they are used. The best we could do would be for the if-block to check that the test does not have entities outside string values and fix them outside strings but then we could not have those raw signs inside strings there without the author using plain text area.

The second problem about the plain text area, i.e. how to deal with file attachments, is also something someone should think about.

And finally the Abacus solution to this is to add a warning to the question name about questions that should not be edited with WYSIWYG editors. Basically, things that have scripts and other funny stuff inside them. But obviously that solution requires the author to name the question correctly and the person editing to understand that warning, both being big problems as most authors generating content witch scripts tend to never have WYSIWYG editors active they quickly forget this as a problem worth mentioning about and on the other side understanding the problem is not something that is obvious to people just starting to edit materials.

jmbragg commented 5 years ago

I thought it might be a bear to make this seamless. Here's some proposed text for a doc's edit then. I'd place this at the end of the Conditional Blocks section of the Question Blocks page in the docs. Sorry I don't have a pull request for you to do this easily...

If you are using a WYSIWYG editor rather than a plain text editor, you're likely to experience problems with greater or less than symbols being converted into &gt; and &lt; by your editor and breaking the CAS evaluation. This could be solved by using a plain text editor, but would require you to remember to never save the question with a WYSIWYG editor again. The easiest solution is to put any tests using > or < into the question variables instead. For example, place the following variable in your question test: blocktest:ev(p<1,simp,pred) And modify your question block to start with: [[ if test="blocktest"]]

That's probably excessively spelled out... Feel free to modify and edit whatever you think is helpful or more concise.

sangwinc commented 5 years ago

We've been having the same problem here in Edinburgh!

Authors edit, and break, perfectly good questions just by editing something trivial (like the question name to keep their question bank tidy.....) It is driving me mad! Good design should prevent this kind of blunder.

So, I need some help finishing off the warning system, which is in place but not something we have yet found a use for.

Matti, can you please contribute a predicate: https://github.com/maths/moodle-qtype_stack/commit/6f65e3dda85562a9e5df7c739476196b5af370c2#diff-43c4c525ee05e2967e5508db573b6f68R1628

Tim, I really need some help getting information through Moodle. I've tried hard and failed! https://github.com/maths/moodle-qtype_stack/commit/6f65e3dda85562a9e5df7c739476196b5af370c2#diff-36a711fa89d596bebb48c2cb0c2fbcd9R771

aharjula commented 5 years ago

Chris, If I understand correctly by predicate you mean something that would identify a case where a warning is required, however if we identify that there are entities within our code it is already too late as in addition to entities we may have all sorts of other "helpful" things elsewhere and we cannot undo that damage at that point. However, if you mean predicate in the sense that the editor would not render a WYSIWYG for content that has some feature then we might have some means of acting early enough, but it might look silly if the editor switches between WYSIWYG and plain every time you do a logic change within inline logic.

Note that this is not a problem that only affects blocks, the same applies for normal {@...@} injections and any scripting authors might want to include in their CASText. For a predicate based on this we could have the following:

  1. Does the CASText contain the substring <script?
  2. Does any block in the text consider itself sensitive to WYSIWYG? i.e. injections with certain chars, blocks with attributes with those chars or blocks that by their very nature are sensitive like the jsxgraph and the foreach block that often leads to mangled HTML if WYSIWYG tries to help within it... This would require an addition to the block-interface but that would not be a big thing.

I would prefer the logic to be so that once that predicate triggers one cannot go back without confirmation, as it feels likely to me that someone might have materials on the limit and could experience behaviour where the WYSIWYG gets to mess up layout with <br>s and other such insanity more than once before they get safely over the limit.

The next issue is to decide whether we do such predicate checking field by field basis or do we toggle all CASText in the whole question at the same time? I would go for the whole question to keep things simple.

Instead of such a predicate and whatever logic gets built around it I would prefer that question level flag to control the editor and override user settings for WYSIWYG when toggled. Or a simple correction to Moodle so that Moodle text editors would not save the result of WYSIWYG processing before the field gets used and would therefore not cause these issues unless the person editing actually goes and messes with the sensitive bits, might solve this issue for others too...

sangwinc commented 5 years ago

Ok Matti, this makes much more sense.

We can use this function https://github.com/maths/moodle-qtype_stack/commit/979ce5222053dad25c1adea49b2bf4a1116b83d4#diff-998aad1a8e8a97437d2c3809d108b746R537 both to trigger a warning, and perhaps also (with more help from Tim (!) ) to override the editor in Moodle and make sure the plain editor is only used. Anyone using scripts etc should not be relying on WYSIWYG editors. That might be annoying, but less so than the current situation.

Chris

timhunt commented 5 years ago

Remember that Moodle supports plug-in-able editors (which by default means the build-in Atto WYSIWYG editor, plain text areas, and the legacy TinyMCE editor).

And, rich-content values like questiontext are stored in two database columns like (questiontext, questiontextformat). The format is one of the constants FORMAT_HTML / FORMAT_MOODLE / FORMAT_PLAIN / FORMAT_MARKDOWN. Some editors only support some formats (e.g. Atto only supports FORMAT_HTML, textarea supports anything.

When a particular user comes to edit a particular bit of content, the editors_get_preferred_editor function (defined in editors_get_preferred_editor) is called to work out which editor to use. That considers:

Since the editor is part of a form, and form rendering is a bit of a black box in Moodle, there are limited ways to get control.

Probably the cleanest way to control this would be to make a new form field type, say 'editorwithstackblocks' (see https://github.com/moodleou/moodle-tool_editrolesbycap/blob/master/capabilityformfield.php for making a new form field type in a plugin). That would be a subclass of MoodleQuickForm_editor which just does the minimal amount of overriding possible. Probably just

function toHtml() {
    global $CFG;
    $savededitorsconfig = $CFG->texteditors;
    if (!use_wysiwyg_editor($text)) {
        $CFG->texteditors = 'textarea';
    }
    $html = parent::toHtml();
    $CFG->texteditorsconfig = $savededitors;
    return $html;
}

That is a nastly global variable hack, but given that the parent toHtml method is a monolithic monster, this is the cleaneast way I can think of to do it. Note, I just typed this off the top of my head. Not tested.

sangwinc commented 5 years ago

Thanks Tim, I think we will implement editorwithstackblocks.

"form rendering is a bit of a black box in Moodle" indeed!

I'm struggling to get the warnings back through the system. Any tips about that please? If I can't solve this, the rest looks a bit hopeless anyway.

Chris

sgparry commented 5 years ago

I think I am having related issues to this - one of the recently added base N options is basen(num, base, "M<") for right justified. It keeps breaking in the question text and feedback panels. Should have figured it was an HTML escape problem. Fortunately my basen function accepts a numeric alternative.

sangwinc commented 4 months ago

This is now a duplicate issue.