Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-42310

Feedback checkboxes, radio buttons and dropdown input controls are not processed through text filter

    Details

    • Testing Instructions:
      Hide

      Prerequisites:

      • Ensure that multilang filter is enabled.
      • The following assumes that your Moodle installation is setup to handle English and French language. You may need to change the languages in the strings to be inserted in order to accommodate the languages you have enabled on your test site.

      As an administrator:

      1. Enable the feedback module going to the admin tree -> plugins -> activity modules -> Manage activities
      2. Go to a course and add a Feedback activity
      3. View the activity
      4. Add a "Multiple choice - single answer" question.
      5. Insert the following text in the label and the value fields: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>
      6. Add a "Multiple choice - multiple answers" question.
      7. Insert the following text in the label and the value fields: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>
      8. Add a "Multiple choice - single answer allowed (dropdownlist)" question.
      9. Insert the following text in the label and the value fields: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>
      10. Add a "Multiple choice (rated) - single answer" question.
      11. Insert the following text in the label field: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>
      12. Insert the following text in the value field: 10/<span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>
      13. Add a "Multiple choice (rated) - multiple answers" question.
      14. Insert the following text in the label field: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>
      15. Insert the following text in the value field: 10/<span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>
      16. Add a "Multiple choice (rated) - single answer allowed (dropdownlist)" question.
      17. Insert the following text in the label field: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>
      18. Insert the following text in the value field: 10/<span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>

      With Editing Mode ON

      Feedback Form Preview - Before the fix, both English and French appear in the control labels (e.g. the text next to a checkbox or radio button or in a dropdown list). After the fix, only the current language strings appear.

      With Editing Mode OFF

      While viewing the Feedback form, Before the fix, notice that both English and French appear in the control labels (e.g. the text next to a checkbox or radio button or in a dropdown list). After the fix, only the current language strings appear.

      In order to make Feedback forms WCAG 2.0 compliant and consistent with language and other filter support, each of the fields must go through the format_string() or format_text() filter function. The fix applies the format_string() function to accomplish this thereby making the filtering of the text consistent.

      The built-in Feedback controls other than those mentioned above were tested and do not have this issue.

      Show
      Prerequisites: Ensure that multilang filter is enabled. The following assumes that your Moodle installation is setup to handle English and French language. You may need to change the languages in the strings to be inserted in order to accommodate the languages you have enabled on your test site. As an administrator: Enable the feedback module going to the admin tree -> plugins -> activity modules -> Manage activities Go to a course and add a Feedback activity View the activity Add a "Multiple choice - single answer" question. Insert the following text in the label and the value fields: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Add a "Multiple choice - multiple answers" question. Insert the following text in the label and the value fields: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Add a "Multiple choice - single answer allowed (dropdownlist)" question. Insert the following text in the label and the value fields: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Add a "Multiple choice (rated) - single answer" question. Insert the following text in the label field: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Insert the following text in the value field: 10/<span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Add a "Multiple choice (rated) - multiple answers" question. Insert the following text in the label field: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Insert the following text in the value field: 10/<span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Add a "Multiple choice (rated) - single answer allowed (dropdownlist)" question. Insert the following text in the label field: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Insert the following text in the value field: 10/<span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> With Editing Mode ON Feedback Form Preview - Before the fix, both English and French appear in the control labels (e.g. the text next to a checkbox or radio button or in a dropdown list). After the fix, only the current language strings appear. With Editing Mode OFF While viewing the Feedback form, Before the fix, notice that both English and French appear in the control labels (e.g. the text next to a checkbox or radio button or in a dropdown list). After the fix, only the current language strings appear. In order to make Feedback forms WCAG 2.0 compliant and consistent with language and other filter support, each of the fields must go through the format_string() or format_text() filter function. The fix applies the format_string() function to accomplish this thereby making the filtering of the text consistent. The built-in Feedback controls other than those mentioned above were tested and do not have this issue.
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull 2.6 Branch:
      wip-MDL-42310-26
    • Pull Master Branch:
      wip-MDL-42310-master

      Description

      The contents of the checkboxes, radio buttons and dropdown input controls are not processed through Moodles format_text() or format_string() filters. As a result, things like multilang filtering is not applied to these fields. This is inconsistent with the field captions and other text displayed by this plugin which appear to all be processed by the format_string() function.

      This issue appears in the multichoice and multichoicerated type of controls.

        Gliffy Diagrams

          Activity

          Hide
          grabs Andreas Grabs added a comment -

          Hi Michael,
          Thank your for your fix! I tried it but unfortunately it didn't work for me .
          The values in the multichoice questions were not filtered at all.
          Instead of format_string($radio) I tried format_text($radio, FORMAT_MOODLE, array('para'=>false)). That worked fine.
          Am I wrong or have I overseen something?
          Best regards
          Andreas

          Show
          grabs Andreas Grabs added a comment - Hi Michael, Thank your for your fix! I tried it but unfortunately it didn't work for me . The values in the multichoice questions were not filtered at all. Instead of format_string($radio) I tried format_text($radio, FORMAT_MOODLE, array('para'=>false)). That worked fine. Am I wrong or have I overseen something? Best regards Andreas
          Hide
          michael-milette Michael Milette added a comment - - edited

          Hi Andreas,

          Thank you so much for looking at this issue. Unfortunately I am at a loss as to why it didn't work for you. I am using the code and it works fine over here.

          According to the Moodle docs:

          http://docs.moodle.org/dev/Output_functions#format_string.28.29

          "...this function is basically one stripped version of the full format_text() function detailed above and it doesn't offer any of its options or protections. It simply filters the strings and returns the result..."

          The documentation recommends using this function on short strings. It is also the function used in other places throughout the feedback plugin where language support has previously been added and I wanted to remain consistent with the coding style elsewhere in the plugin.

          It is possible that you are experiencing a caching issue? If not, could you paste a specific block of code (let me know the line number too) and I will try to spot why it isn't working.

          Thanks again for your effort and your time. It is much appreciated.

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - - edited Hi Andreas, Thank you so much for looking at this issue. Unfortunately I am at a loss as to why it didn't work for you. I am using the code and it works fine over here. According to the Moodle docs: http://docs.moodle.org/dev/Output_functions#format_string.28.29 "...this function is basically one stripped version of the full format_text() function detailed above and it doesn't offer any of its options or protections. It simply filters the strings and returns the result..." The documentation recommends using this function on short strings. It is also the function used in other places throughout the feedback plugin where language support has previously been added and I wanted to remain consistent with the coding style elsewhere in the plugin. It is possible that you are experiencing a caching issue? If not, could you paste a specific block of code (let me know the line number too) and I will try to spot why it isn't working. Thanks again for your effort and your time. It is much appreciated. Best regards, Michael
          Hide
          grabs Andreas Grabs added a comment -

          Hi Michael,

          I am sure there is no caching issue. I could reproduce it.
          I tried it in function print_item_radio().
          In line 694 your code is:
          <?php echo text_to_html(format_string($radio), true, false, false);?> 
          but was not working.
          I tried
          <?php echo text_to_html(format_text($radio, FORMAT_MOODLE, array('para'=>false)), true, false, false);?> 
          and it worked.
          I don't know why it doesn't work .
          I will give it some try on another server but not today.

          Best regards
          Andreas

          Show
          grabs Andreas Grabs added a comment - Hi Michael, I am sure there is no caching issue. I could reproduce it. I tried it in function print_item_radio(). In line 694 your code is: <?php echo text_to_html(format_string($radio), true, false, false);?>  but was not working. I tried <?php echo text_to_html(format_text($radio, FORMAT_MOODLE, array('para'=>false)), true, false, false);?>  and it worked. I don't know why it doesn't work . I will give it some try on another server but not today. Best regards Andreas
          Hide
          michael-milette Michael Milette added a comment -

          Hi Andreas,

          When I first tried the format_string() function back in June in Moodle 2.5+, I recall that this function didn't work for me either. It was my first time using it and I just assumed that I wasn't using it correctly so I ended up using format_text() instead. Maybe there was actually a bug in 2.5+ which has since been fixed in 2.5.1 which is what I am testing on today.

          Just to confirm that I was testing the right section of code with a sample question, I tested the format_string() function and even made an obvious changes to that line of code which was reflected in the rendered page and everything worked. I can flip back and forth between English and French and it consistently works correctly.

          Here is an idea: Try adding an Information type field to your form. The code for this field type was previously updated by someone else in the past. It uses the format_string() function as well. Then put the following line in the Question field:

          <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span>

          Then view your form. If it has the same problem, them it could likely be the version of Moodle you are using. In that case, a cross version solution would be to use the format_text() function everywhere including the Information field as you described in your original message.

          On the other hand, if it does work correctly, them we have to wonder why the function works in one place and not the other.

          Let me know how it turns out for you.

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - Hi Andreas, When I first tried the format_string() function back in June in Moodle 2.5+, I recall that this function didn't work for me either. It was my first time using it and I just assumed that I wasn't using it correctly so I ended up using format_text() instead. Maybe there was actually a bug in 2.5+ which has since been fixed in 2.5.1 which is what I am testing on today. Just to confirm that I was testing the right section of code with a sample question, I tested the format_string() function and even made an obvious changes to that line of code which was reflected in the rendered page and everything worked. I can flip back and forth between English and French and it consistently works correctly. Here is an idea: Try adding an Information type field to your form. The code for this field type was previously updated by someone else in the past. It uses the format_string() function as well. Then put the following line in the Question field: <span lang="en" class="multilang">English</span><span lang="fr" class="multilang">French</span> Then view your form. If it has the same problem, them it could likely be the version of Moodle you are using. In that case, a cross version solution would be to use the format_text() function everywhere including the Information field as you described in your original message. On the other hand, if it does work correctly, them we have to wonder why the function works in one place and not the other. Let me know how it turns out for you. Best regards, Michael
          Hide
          salvetore Michael de Raadt added a comment -

          Thanks for working on this, guys.

          Show
          salvetore Michael de Raadt added a comment - Thanks for working on this, guys.
          Hide
          grabs Andreas Grabs added a comment -

          Hi Michael,

          I don't sure that I fully understand you.
          Now I had been tested on different machines and with moodle 2.6dev and 2.5.2.
          The function format_string() does not use any filter at all. No emoticons or some other stuff were processed.
          The only thing what was filtered in the way we want was the question text. But this is filtered by format_text().
          I have no idea what the reason could be .

          Best regards
          Andreas

          Show
          grabs Andreas Grabs added a comment - Hi Michael, I don't sure that I fully understand you. Now I had been tested on different machines and with moodle 2.6dev and 2.5.2. The function format_string() does not use any filter at all. No emoticons or some other stuff were processed. The only thing what was filtered in the way we want was the question text. But this is filtered by format_text(). I have no idea what the reason could be . Best regards Andreas
          Hide
          michael-milette Michael Milette added a comment -

          Using format_text() instead of format_string()

          Show
          michael-milette Michael Milette added a comment - Using format_text() instead of format_string()
          Hide
          michael-milette Michael Milette added a comment -

          Hi Andreas,

          I've submitted a second solution (see links) where I applied the format_text() function instead of the format_string() function. Let me know if the patch gives you any more grief.

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - Hi Andreas, I've submitted a second solution (see links) where I applied the format_text() function instead of the format_string() function. Let me know if the patch gives you any more grief. Best regards, Michael
          Hide
          grabs Andreas Grabs added a comment -

          Hi Michael,

          thank you for your patience!
          Now it works as it should .
          Thank you for your patch.

          Best regards
          Andreas

          Show
          grabs Andreas Grabs added a comment - Hi Michael, thank you for your patience! Now it works as it should . Thank you for your patch. Best regards Andreas
          Hide
          michael-milette Michael Milette added a comment -

          Thank you Andreas, have a great day!

          Show
          michael-milette Michael Milette added a comment - Thank you Andreas, have a great day!
          Hide
          damyon Damyon Wiese added a comment -

          Thanks for working on this issue - but it is not quite right yet.

          format_text is being called wrongly in this patch.

          Here are the docs for the arguments to the function:

           * @param string $text The text to be formatted. This is raw text originally from user input.                                       
           * @param int $format Identifier of the text format to be used                                                                      
           *            [FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN, FORMAT_MARKDOWN]                                                           
           * @param object/array $options text formatting options                                                                             
           * @param int $courseiddonotuse deprecated course id, use context option instead
          

          You are calling this function like format_text($blah, true, false, false)
          It is working for you - but only by accident and may break in future.
          Please pass the proper arguments, or omit them (you are using the defaults anyway).

          Reopening this issue for more work.

          Show
          damyon Damyon Wiese added a comment - Thanks for working on this issue - but it is not quite right yet. format_text is being called wrongly in this patch. Here are the docs for the arguments to the function: * @param string $text The text to be formatted. This is raw text originally from user input. * @param int $format Identifier of the text format to be used * [FORMAT_MOODLE, FORMAT_HTML, FORMAT_PLAIN, FORMAT_MARKDOWN] * @param object/array $options text formatting options * @param int $courseiddonotuse deprecated course id, use context option instead You are calling this function like format_text($blah, true, false, false) It is working for you - but only by accident and may break in future. Please pass the proper arguments, or omit them (you are using the defaults anyway). Reopening this issue for more work.
          Hide
          michael-milette Michael Milette added a comment - - edited

          Hi Damyon,

          Thank you for your feedback.

          I fixed the calls to format_text() as requested. I also fixed all of the other instances where format_text() was being called with similar incorrect parameters in multichoice and multichoicerated. I wasn't sure where to paste the URL for the new diff so I put it in the Pull Master Diff URL field above. Hope this was ok.

          By the time you read this, I will have also upload a WCAG sample feedback form XML file which you can import into Moodle to make it easier to setup testing. While this tracker issue only deals with the multichoice and multichoicerated input controls, the sample form includes various configurations for all of the input control types. Hope this helps with your testing.

          Note that the sample file tests the format_text function by using the multilang filter. It is setup to test English and French. If you are setup for different languages, it might be easier and quicker to just do a search and replace on the XML file before you import it in order to match the languages you have installed on your Moodle installation.

          Let me know if you need any other changes.

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - - edited Hi Damyon, Thank you for your feedback. I fixed the calls to format_text() as requested. I also fixed all of the other instances where format_text() was being called with similar incorrect parameters in multichoice and multichoicerated. I wasn't sure where to paste the URL for the new diff so I put it in the Pull Master Diff URL field above. Hope this was ok. By the time you read this, I will have also upload a WCAG sample feedback form XML file which you can import into Moodle to make it easier to setup testing. While this tracker issue only deals with the multichoice and multichoicerated input controls, the sample form includes various configurations for all of the input control types. Hope this helps with your testing. Note that the sample file tests the format_text function by using the multilang filter. It is setup to test English and French. If you are setup for different languages, it might be easier and quicker to just do a search and replace on the XML file before you import it in order to match the languages you have installed on your Moodle installation. Let me know if you need any other changes. Best regards, Michael
          Hide
          grabs Andreas Grabs added a comment -

          Hi Michael,

          unfortunately I can't neither merge nor cherry-pick your commit. Maybe you have to update your repository and rebase your branches.

          Best regards
          Andreas

          Show
          grabs Andreas Grabs added a comment - Hi Michael, unfortunately I can't neither merge nor cherry-pick your commit. Maybe you have to update your repository and rebase your branches. Best regards Andreas
          Hide
          michael-milette Michael Milette added a comment -

          Hi Andreas,

          I am not too familiar with the process of updating my repository and rebasing my branches however I think I did it properly. I updated the Pull Master Diff URL again. Hopefully it will work better now.

          If it sill doesn't work, any guidance as to what I need to do would be much appreciated.

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - Hi Andreas, I am not too familiar with the process of updating my repository and rebasing my branches however I think I did it properly. I updated the Pull Master Diff URL again. Hopefully it will work better now. If it sill doesn't work, any guidance as to what I need to do would be much appreciated. Best regards, Michael
          Hide
          grabs Andreas Grabs added a comment - - edited

          Hi Michael,

          I am sorry but I can't still merge.
          I found two branches to this issue but non of them fits.

          MDL-42310-Feedback-Input-Fields-Not-Filtered
          MDL-42310-Feedback-Input-Fields-Not-Filtered-fix-v2

          Try to rebase these branches to the current master.
          Maybe that ressource helps you: http://git-scm.com/book/en/Git-Branching-Rebasing

          Best regards
          Andreas

          Show
          grabs Andreas Grabs added a comment - - edited Hi Michael, I am sorry but I can't still merge. I found two branches to this issue but non of them fits. MDL-42310 -Feedback-Input-Fields-Not-Filtered MDL-42310 -Feedback-Input-Fields-Not-Filtered-fix-v2 Try to rebase these branches to the current master. Maybe that ressource helps you: http://git-scm.com/book/en/Git-Branching-Rebasing Best regards Andreas
          Hide
          michael-milette Michael Milette added a comment -

          I deleted all of the old patches and commits and recreated the one for MDL-42310 from scratch. Hopefully everything will work now.

          I updated the link in the Pull Master Diff URL field.

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - I deleted all of the old patches and commits and recreated the one for MDL-42310 from scratch. Hopefully everything will work now. I updated the link in the Pull Master Diff URL field. Best regards, Michael
          Hide
          grabs Andreas Grabs added a comment - - edited

          Hi Michael,

          unfortunately your repository still doesn't fit .
          I would suggest you create a new branch from the current master:

          git checkout -b MDL-42310_master master

          Then change the code in the way you want and commit them and push the new branch to github.
          Later if you have to do more changes for the same issue so you don't need to create a new branch.
          Just do the changes in the branch "MDL-42310_master" and commit and push them to github.
          Now the branch has all commits that belong the issue.
          The diff-url should be a url that compare your HEAD of MDL-42310_master with the HEAD of master. It could look like that:
          https://github.com/grabs/moodle/compare/master...MDL-42310_master

          I hope it helps a little.

          Best regards
          Andreas

          Show
          grabs Andreas Grabs added a comment - - edited Hi Michael, unfortunately your repository still doesn't fit . I would suggest you create a new branch from the current master: git checkout -b MDL-42310 _master master Then change the code in the way you want and commit them and push the new branch to github. Later if you have to do more changes for the same issue so you don't need to create a new branch. Just do the changes in the branch " MDL-42310 _master" and commit and push them to github. Now the branch has all commits that belong the issue. The diff-url should be a url that compare your HEAD of MDL-42310 _master with the HEAD of master. It could look like that: https://github.com/grabs/moodle/compare/master...MDL-42310_master I hope it helps a little. Best regards Andreas
          Hide
          michael-milette Michael Milette added a comment -

          Hi Andreas,

          Thank you for the instructions and your patience. I completed the instruction as indicated and update the Pull Master Diff URL. I discovered that my upstream and the origin were seriously out of sync and had to sync up origin, upstream and master and then apply the changes.

          I've updated the URL in Pull Master Diff URL. Hope it works better for you.

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - Hi Andreas, Thank you for the instructions and your patience. I completed the instruction as indicated and update the Pull Master Diff URL. I discovered that my upstream and the origin were seriously out of sync and had to sync up origin, upstream and master and then apply the changes. I've updated the URL in Pull Master Diff URL. Hope it works better for you. Best regards, Michael
          Hide
          michael-milette Michael Milette added a comment -

          Updated the test form. One of the questions had the language settings in reverse (was showing French on English side of the site).

          Show
          michael-milette Michael Milette added a comment - Updated the test form. One of the questions had the language settings in reverse (was showing French on English side of the site).
          Hide
          grabs Andreas Grabs added a comment -

          Hi Michael,
          thank you for your work .
          Now I could integrate and test your changes.
          Best regards
          Andreas

          Show
          grabs Andreas Grabs added a comment - Hi Michael, thank you for your work . Now I could integrate and test your changes. Best regards Andreas
          Hide
          michael-milette Michael Milette added a comment -

          Thanks. Glad to hear I finally got it

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - Thanks. Glad to hear I finally got it Best regards, Michael
          Hide
          michael-milette Michael Milette added a comment -

          Is there any chance that this might make it into the 2.6.1 release?

          Show
          michael-milette Michael Milette added a comment - Is there any chance that this might make it into the 2.6.1 release?
          Hide
          abgreeve Adrian Greeve added a comment -

          Hello Michael,

          I think that the code for this looks good.
          If you could just tidy up the code so that arrows in arrays have spaces.
          e.g. array('noclean' => true, 'para' => false)
          And Could you also "squash" your commits into one then we should be ready for integration.

          Kind regards,

          Adrian.

          P.S. squashing your commits can be done by doing the following:

          • git rebase -i origin/master
          • deleting "pick" on the second commit and replacing it with just "f"
          • save the file
          Show
          abgreeve Adrian Greeve added a comment - Hello Michael, I think that the code for this looks good. If you could just tidy up the code so that arrows in arrays have spaces. e.g. array('noclean' => true, 'para' => false) And Could you also "squash" your commits into one then we should be ready for integration. Kind regards, Adrian. P.S. squashing your commits can be done by doing the following: git rebase -i origin/master deleting "pick" on the second commit and replacing it with just "f" save the file
          Hide
          michael-milette Michael Milette added a comment -

          Hi Adrian,

          Thanks for the feedback.

          I made the changes you requested and updated the Pull Master Diff URL but didn't have to rebase because I had to re-create the branch from scratch after loosing all my branches while trying to rebase a couple of weeks ago when another of my patches was being published.

          I have yet to figure out a way of synchronizing my local master repo with the official Moodle master repo without occasionally messing up so badly that I end up in my having to recreate everything. Somehow, it never seems to work out as easily as described on the Git for Developers page.

          Anyway, hope this meets with your eagle eye approval (which is always appreciated... thanks!). If not, please let me know and I will be happy to address your concerns.

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - Hi Adrian, Thanks for the feedback. I made the changes you requested and updated the Pull Master Diff URL but didn't have to rebase because I had to re-create the branch from scratch after loosing all my branches while trying to rebase a couple of weeks ago when another of my patches was being published. I have yet to figure out a way of synchronizing my local master repo with the official Moodle master repo without occasionally messing up so badly that I end up in my having to recreate everything. Somehow, it never seems to work out as easily as described on the Git for Developers page. Anyway, hope this meets with your eagle eye approval (which is always appreciated... thanks!). If not, please let me know and I will be happy to address your concerns. Best regards, Michael
          Hide
          abgreeve Adrian Greeve added a comment -

          Hi Michael,

          Thank you for the update. I'm sorry to hear that you are having difficulties with your git repositories. I remember a few situations where I've had to blow away my repositories and start again. Thankfully that hasn't been recently.

          I know that the formatting of the code seems like an incredibly minor thing, and I realise that it can be annoying to have progress halted due to these small seemingly insignificant details. The integration team here is pretty fussy about the code that is introduced into core. Sometimes they will run code checking software to make sure that everything meets the coding guidlines and so everyone here at HQ has gotten into the habit of picking up these formatting discrepancies. I thank you for your patience.

          I think that your code is ready to go. Can you provide branches for versions 2.4 - 2.6? I think that this issue could be back-ported.

          I hope that you had a happy new year.

          Adrian.

          Show
          abgreeve Adrian Greeve added a comment - Hi Michael, Thank you for the update. I'm sorry to hear that you are having difficulties with your git repositories. I remember a few situations where I've had to blow away my repositories and start again. Thankfully that hasn't been recently. I know that the formatting of the code seems like an incredibly minor thing, and I realise that it can be annoying to have progress halted due to these small seemingly insignificant details. The integration team here is pretty fussy about the code that is introduced into core. Sometimes they will run code checking software to make sure that everything meets the coding guidlines and so everyone here at HQ has gotten into the habit of picking up these formatting discrepancies. I thank you for your patience. I think that your code is ready to go. Can you provide branches for versions 2.4 - 2.6? I think that this issue could be back-ported. I hope that you had a happy new year. Adrian.
          Hide
          michael-milette Michael Milette added a comment -

          Hi Adrian,

          Happy New Year!

          With regards to the formatting, it's not a problem. Consistent coding style helps all developers, especially when you are searching for specific code. Yes, Git can be a pain at times but I understand.

          As for branches for v2.4 to 2.6, I am not sure how to do that as the Git GUI tools only show the "master" branch, not the other branches and I don't have the spare cycles to look into that at the moment. Is there some easy automated way to do this?

          Best regards,

          Michael

          Show
          michael-milette Michael Milette added a comment - Hi Adrian, Happy New Year! With regards to the formatting, it's not a problem. Consistent coding style helps all developers, especially when you are searching for specific code. Yes, Git can be a pain at times but I understand. As for branches for v2.4 to 2.6, I am not sure how to do that as the Git GUI tools only show the "master" branch, not the other branches and I don't have the spare cycles to look into that at the moment. Is there some easy automated way to do this? Best regards, Michael
          Hide
          abgreeve Adrian Greeve added a comment -

          Hi Michael,

          To create fixes for the different branches, you effectively need to install a version of each branch on your machine. When this is done you can cherry-pick the fix from one branch to another.

          I've created branches for each of the different branches for you and submitted for integration review.

          Show
          abgreeve Adrian Greeve added a comment - Hi Michael, To create fixes for the different branches, you effectively need to install a version of each branch on your machine. When this is done you can cherry-pick the fix from one branch to another. I've created branches for each of the different branches for you and submitted for integration review.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Really all those calls should be also passing the context in the $options, instead of relying on format_text() own abilities to fallback to $PAGE->context, just imagine a place where you want to use those classes without being "attached" to any $PAGE... for sure there will be debug warnings about missing context (CLI...).

          Anyway, this change is a step towards better use if format_text() occurrences... so integrating it...

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Really all those calls should be also passing the context in the $options, instead of relying on format_text() own abilities to fallback to $PAGE->context, just imagine a place where you want to use those classes without being "attached" to any $PAGE... for sure there will be debug warnings about missing context (CLI...). Anyway, this change is a step towards better use if format_text() occurrences... so integrating it...
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated (24, 25, 26 & 27), thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (24, 25, 26 & 27), thanks!
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Hi Adrian and Michael,

          The multilang filter for the label fields is not working properly. Instead of filtering the text base on the language, it truncates the tag and and quote for the field.

          This is the output value for the label field:

          spanlangenclassmultilangEnglishspanspanlangfrclassmultilangFrenchspan
          

          Also, there is no multiple answer option type in multiple choice (rated) question.

          Please confirm/clarify the above comments.

          Failing test.

          Show
          rwijaya Rossiani Wijaya added a comment - Hi Adrian and Michael, The multilang filter for the label fields is not working properly. Instead of filtering the text base on the language, it truncates the tag and and quote for the field. This is the output value for the label field: spanlangenclassmultilangEnglishspanspanlangfrclassmultilangFrenchspan Also, there is no multiple answer option type in multiple choice (rated) question. Please confirm/clarify the above comments. Failing test.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          uhm... this issue was only about multichoice and multichoicerated elements. Looking at testing instructions I just discovered it includes a test with a label.

          1) The label test is unrelated with this.
          2) Looking to code, it seems that labels would work ok if $CFG->enabletrusttext=true is set. I think that's a wrong condition, but that's another issue.

          So I'd recommend to (any of these):

          a) or you ignore the labels test completely.
          b) or you enable $CFG->enabletrusttext and perform the labels test.

          Sending this back to testing, hope Michael can confirm the label test is unrelated, ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - uhm... this issue was only about multichoice and multichoicerated elements. Looking at testing instructions I just discovered it includes a test with a label. 1) The label test is unrelated with this. 2) Looking to code, it seems that labels would work ok if $CFG->enabletrusttext=true is set. I think that's a wrong condition, but that's another issue. So I'd recommend to (any of these): a) or you ignore the labels test completely. b) or you enable $CFG->enabletrusttext and perform the labels test. Sending this back to testing, hope Michael can confirm the label test is unrelated, ciao
          Hide
          rwijaya Rossiani Wijaya added a comment -

          Thanks Eloy for the comments.

          Re-tested this again by ignoring the label test and the options displayed as expected with the language pack.

          As for the multiple answers option type in multiple choice (rated) question, I think it is just an invalid testing instruction. Perhaps Michael or Adrian could confirm this.

          Testing this issue for 2.4, 2.5, 2.6 and master.

          Test passed.

          Show
          rwijaya Rossiani Wijaya added a comment - Thanks Eloy for the comments. Re-tested this again by ignoring the label test and the options displayed as expected with the language pack. As for the multiple answers option type in multiple choice (rated) question, I think it is just an invalid testing instruction. Perhaps Michael or Adrian could confirm this. Testing this issue for 2.4, 2.5, 2.6 and master. Test passed.
          Hide
          michael-milette Michael Milette added a comment -

          Thanks Eloy and Rossiani.

          Indeed, the label test is not related. But if you want to get it to work, I suspect that you may need to make the following configuration change in order to enable filtering on the labels as well:

          1. Log in as an administrator
          2. Click Home > Site Administration > Plugins > Filters > Manage Filters
          3. Locate the Multi-Language filter and ensure a) that it is on, which it probably is already based on your comments, and b) that Apply to is set to Content and Headings, not just Content which is the default.

          I've found myself stuck on this one from time to time when I forget about this setting on new installations. I am not sure why the default is to only filter content but making the above change should fix your problem. You should not need to set $CFG->enabletrsusttext=true in order to get labels to work.

          Let me know if you have any more concerns.

          Best regards,

          Michael Milete

          Show
          michael-milette Michael Milette added a comment - Thanks Eloy and Rossiani. Indeed, the label test is not related. But if you want to get it to work, I suspect that you may need to make the following configuration change in order to enable filtering on the labels as well: Log in as an administrator Click Home > Site Administration > Plugins > Filters > Manage Filters Locate the Multi-Language filter and ensure a) that it is on, which it probably is already based on your comments, and b) that Apply to is set to Content and Headings , not just Content which is the default. I've found myself stuck on this one from time to time when I forget about this setting on new installations. I am not sure why the default is to only filter content but making the above change should fix your problem. You should not need to set $CFG->enabletrsusttext=true in order to get labels to work. Let me know if you have any more concerns. Best regards, Michael Milete
          Hide
          damyon Damyon Wiese added a comment -

          David built a framework for behat
          At first just to test this and that
          10000+ steps written
          Sounds like we're all smitten
          And David should be smiling at that

          Thanks for reporting, patching, and testing this issue. It has been released upstream along with 64 others today.

          Show
          damyon Damyon Wiese added a comment - David built a framework for behat At first just to test this and that 10000+ steps written Sounds like we're all smitten And David should be smiling at that Thanks for reporting, patching, and testing this issue. It has been released upstream along with 64 others today.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                13/Jan/14