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

Feedback - Fix for Accessibility Issue with Informational and Error Content Displayed in Red

    Details

    • Testing Instructions:
      Hide

      Use attached feedback form (exported file) specifically designed for testing WCAG 2.0 compliance (and multilang filter support but we are not concerned with this here).

      The attached Feedback form assumes the site is configured for English (lang="en") and French (lang="fr"). If this is not the case, you will need to modify the XML file before importing it, modify the fields after it has been imported into Moodle.

      There are two places to verify: 1) When editing the list of questions and 2) when completing the form.

      Things to notice include:

      1. The string "There are required fields in this form marked" followed by a graphic of a red asterisk with the ALT/Title text: "Required field". You can verify this in the source code. You can also just hover your mouse over the asterisk and notice that it now says "Required field" instead of just "required" (not supported in all browsers but works in Chrome). For comparison, this is the same ALT/Title text and image which appears in the profile editor.
      2. The string "Saving failed because missing or false values", which appears at the top of the form if a submission fails due to incorrect or missing field data, is the same text formatting and image as #1 above.
      3. The asterisk next to each required fields is now the same graphic mentioned above with the same ALT/Title text. You can verify this in the source code. You can also just hover your mouse over the asterisk and notice that it now says "Required field" instead of just "required" (not supported in all browsers but works in Chrome). For comparison, this is the same ALT/Title text and image which appears in the profile editor.
      4. If a required field is not completed correctly, upon submission a message is now displayed right above the field instead of the field label itself just changing colour. The styles used are the same ones used in other Moodle forms.
      5. Switching to a theme that modifies any of the following classes will result in formatting changes to the above mentioned strings. Sorry, I don't have any examples.

      These changes make the Feedback form in a much more similar way to other Moodle forms.

      Show
      Use attached feedback form (exported file) specifically designed for testing WCAG 2.0 compliance (and multilang filter support but we are not concerned with this here). The attached Feedback form assumes the site is configured for English (lang="en") and French (lang="fr"). If this is not the case, you will need to modify the XML file before importing it, modify the fields after it has been imported into Moodle. There are two places to verify: 1) When editing the list of questions and 2) when completing the form. Things to notice include: The string "There are required fields in this form marked" followed by a graphic of a red asterisk with the ALT/Title text: "Required field". You can verify this in the source code. You can also just hover your mouse over the asterisk and notice that it now says "Required field" instead of just "required" (not supported in all browsers but works in Chrome). For comparison, this is the same ALT/Title text and image which appears in the profile editor. The string "Saving failed because missing or false values", which appears at the top of the form if a submission fails due to incorrect or missing field data, is the same text formatting and image as #1 above. The asterisk next to each required fields is now the same graphic mentioned above with the same ALT/Title text. You can verify this in the source code. You can also just hover your mouse over the asterisk and notice that it now says "Required field" instead of just "required" (not supported in all browsers but works in Chrome). For comparison, this is the same ALT/Title text and image which appears in the profile editor. If a required field is not completed correctly, upon submission a message is now displayed right above the field instead of the field label itself just changing colour. The styles used are the same ones used in other Moodle forms. Switching to a theme that modifies any of the following classes will result in formatting changes to the above mentioned strings. Sorry, I don't have any examples. These changes make the Feedback form in a much more similar way to other Moodle forms.
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-42462-Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red

      Description

      The following Feedback form elements fail to meet WCAG 2.0 requirements 1.4.3, 1.4.1 and 3.3.2:

      • The string "(*)Answers are required to starred questions." at the top of the form.
      • The string "Saving failed because missing or false values" which appears at the top of the form if submission fails due to incorrect or missing field data.
      • The asterisk appearing at the end of each required question.

      Also, the text "Answers are required to starred questions" is not clear.

      The meaning of the asterisk is not clearly explained and there is no space between the text and the asterisk causing the form to fail WCAG 2.0 success criterion 3.2.2 (3:Understandable, 3:Input Assistance, 2:Labels or Instructions).

      The contrast level between text with a red foreground colour (color: #ff0000 over #FFFFFF) fail WCAG 2.0 success criterion 1.4.3 (1:Perceivable, 4:Distinguishable, 3:Contrast Minimum).

      Recommendation:

      1) Replace the " Answers are required to starred questions." string with the same code used in other Moodle forms to display the "There are required fields in this form marked (*/Required field)" message instead

      2) Replace the text based asterisk for required fields with the same graphical asterisk used in other Moodle forms.

      3) Eliminate the Feedback module styles (missingrequire, feeback_required_mark) that are currently used to highlight required and error fields. Apply the common Moodle styles (mform, fdescription, required, req and error) instead which are used throughout the rest of Moodle.

      4) When the submitted form fails to save for any reason, either "Saving failed because missing or false values" or simply "Saving failed" appears at the top of the form. This text will now be formatted using the same dark red on light red style (span.error) used by other error messages throughout the form.

      This will result in a more uniform appearance of the feedback form which will be consistent with other Moodle forms. The colour issues mentioned above will no longer be caused by the Feedback module as colours will be controlled by Moodle's "Moodle forms" related style sheets. These are easily customizable to meet individual themes requirements.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            michael-milette Michael Milette added a comment -

            Modifies CSS code in:

            • /mod/feedback/styles.css
            • /theme/standard/style/core.css
            • /theme/standard/style/modules.css

            Colours were selected to be as close as possible to the original colours while achieving the required contrast ratio.

            Show
            michael-milette Michael Milette added a comment - Modifies CSS code in: /mod/feedback/styles.css /theme/standard/style/core.css /theme/standard/style/modules.css Colours were selected to be as close as possible to the original colours while achieving the required contrast ratio.
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for working on this, Michael.

            It would be better if you could use the Edit function to add your Git branches in specific fields and your testing instructions in the appropriate area.

            Show
            salvetore Michael de Raadt added a comment - Thanks for working on this, Michael. It would be better if you could use the Edit function to add your Git branches in specific fields and your testing instructions in the appropriate area.
            Hide
            bawjaws David Scotson added a comment -

            The theme/standard/style/core.css also defines:

            271:.mform .required

            color: Color value is invalid

            274:.mform .fdescription.required

            color: Color value is invalid

            It would be nice if Feedback (and all the other modules that don't use mform to generate forms) could re-use this CSS in some way so that all "required" fields use the same shade of red and it's only defined once in the CSS.

            (As a side note, it's defined twice in that code snippet above, and the second definition of color seems to be unnecessary, so it could be deleted.)

            Then if, for example, there's an accessibility issue with a color you only have to find and fix it once.

            There's also the issue that two of the changes are to Standard theme, so most themes (which inherit from Standard's parent theme Base, and not Standard directly) will not see this improvement. I've noticed there's quite a few "colors" that are only present in Standard, which is fine if they're just for decoration as it saves overriding them in child themes, but as in this example, sometimes they have a meaning too.

            Show
            bawjaws David Scotson added a comment - The theme/standard/style/core.css also defines: 271:.mform .required color: Color value is invalid 274:.mform .fdescription.required color: Color value is invalid It would be nice if Feedback (and all the other modules that don't use mform to generate forms) could re-use this CSS in some way so that all "required" fields use the same shade of red and it's only defined once in the CSS. (As a side note, it's defined twice in that code snippet above, and the second definition of color seems to be unnecessary, so it could be deleted.) Then if, for example, there's an accessibility issue with a color you only have to find and fix it once. There's also the issue that two of the changes are to Standard theme, so most themes (which inherit from Standard's parent theme Base, and not Standard directly) will not see this improvement. I've noticed there's quite a few "colors" that are only present in Standard, which is fine if they're just for decoration as it saves overriding them in child themes, but as in this example, sometimes they have a meaning too.
            Hide
            bawjaws David Scotson added a comment -

            Hmm, something odd happened to the code I pasted in above. I also got the location wrong, it's from the Base theme, not Standard so here's a second go with the curly brackets removed:

            theme/base/style/core.css
            271:.mform .required color:#A00;
            274:.mform .fdescription.required color:#A00;text-align:right;

            Show
            bawjaws David Scotson added a comment - Hmm, something odd happened to the code I pasted in above. I also got the location wrong, it's from the Base theme, not Standard so here's a second go with the curly brackets removed: theme/base/style/core.css 271:.mform .required color:#A00; 274:.mform .fdescription.required color:#A00;text-align:right;
            Hide
            michael-milette Michael Milette added a comment -

            Please let me know if any further action on my part is required related to this issue.

            Show
            michael-milette Michael Milette added a comment - Please let me know if any further action on my part is required related to this issue.
            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
            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
            michael-milette Michael Milette added a comment - - edited

            Hi Andreas,

            Is there anything more that I need to do in order to get this issue resolved?

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - - edited Hi Andreas, Is there anything more that I need to do in order to get this issue resolved? Best regards, Michael
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            • Error: the repository field is empty. Nothing was checked.
            Show
            cibot CiBoT added a comment - Results for MDL-42462 Error: the repository field is empty. Nothing was checked.
            Hide
            michael-milette Michael Milette added a comment - - edited

            Hi CiBoT,

            What does this error mean? Do I need to do anything? I have submitted several patches in the past exactly the same way but haven't come across this.

            I Googled the error but all that came up was the same comment you added to several tracker issues (mine and others) today.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - - edited Hi CiBoT, What does this error mean? Do I need to do anything? I have submitted several patches in the past exactly the same way but haven't come across this. I Googled the error but all that came up was the same comment you added to several tracker issues (mine and others) today. Best regards, Michael
            Hide
            michael-milette Michael Milette added a comment -

            I just added the Pull from Repository URL.

            Please let me know if there is any other required information missing.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - I just added the Pull from Repository URL. Please let me know if there is any other required information missing. Best regards, Michael
            Hide
            salvetore Michael de Raadt added a comment -

            I've unassigned Andreas as peer reviewer so that anyone else can take on the review. If Andreas comes soon, he can peer review it, otherwise it's open.

            Show
            salvetore Michael de Raadt added a comment - I've unassigned Andreas as peer reviewer so that anyone else can take on the review. If Andreas comes soon, he can peer review it, otherwise it's open.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks for working on this Michael,

            I think this issue should have been split into 2 as you are really making to separate changes here. Also this needs rebasing because it is conflicting with master right now.

            Note: Accessibility issues are very difficult and thanks for having a go at this one. I think this needs some more work and more thought though - comments below:

            First - the colours. This fix only fixes the non-bootstrap themes, and worse even, only the ones based on standard. This must be fixed on all or none of the core themes.

            Your CSS changes include TABS in the source which is not allowed according to the moodle coding guidelines.

            Depending on whether we want to support AA or AAA this choice of color may be a bit extreme, (even #ee0000 is acceptable for AA).

            Your solution for the abbr tags seems a bit wrong for me. Blind users would presumably hear: "Required, items are required" - but even the example you linked does something different (required items are marked with a *).

            Our solution for this in normal moodle forms is different - and we should use the same markup everywhere. Ideally all these feedback forms could be moodle forms and then we would have no issue.

            In a normal moodle form, we use an image with alt text for the asterix. IMO this solution should match that one.

            That's all I have.

            Thanks again, Damyon

            Show
            damyon Damyon Wiese added a comment - Thanks for working on this Michael, I think this issue should have been split into 2 as you are really making to separate changes here. Also this needs rebasing because it is conflicting with master right now. Note: Accessibility issues are very difficult and thanks for having a go at this one. I think this needs some more work and more thought though - comments below: First - the colours. This fix only fixes the non-bootstrap themes, and worse even, only the ones based on standard. This must be fixed on all or none of the core themes. Your CSS changes include TABS in the source which is not allowed according to the moodle coding guidelines. Depending on whether we want to support AA or AAA this choice of color may be a bit extreme, (even #ee0000 is acceptable for AA). Your solution for the abbr tags seems a bit wrong for me. Blind users would presumably hear: "Required, items are required" - but even the example you linked does something different (required items are marked with a *). Our solution for this in normal moodle forms is different - and we should use the same markup everywhere. Ideally all these feedback forms could be moodle forms and then we would have no issue. In a normal moodle form, we use an image with alt text for the asterix. IMO this solution should match that one. That's all I have. Thanks again, Damyon
            Hide
            michael-milette Michael Milette added a comment - - edited

            Hi Damyon,

            Thank you for your feedback. I can see that you've put a lot of thought into this. I will address the colour contrast ratio in this message and follow up with your other concerns in separate comments.

            The following success criterions are relevant here depending on whether you are aiming for compliance with level AA or AAA. Our client, the Government of Canada requires WCAG 2.0 level AA compliance.

            SC 1.4.3 Contrast (Minimum): The visual presentation of text and images of text has a contrast ratio of at least 4.5:1, except for the following: (Level AA) Large Print: Large-scale text and images of large-scale text have a contrast ratio of at least 3:1.

            SC 1.4.6 Contrast (Enhanced): The visual presentation of text and images of text has a contrast ratio of at least 7:1, except for the following: (Level AAA) Large Print: Large-scale text and images of large-scale text have a contrast ratio of at least 4.5:1.

            It is important to note in this situation that making the text bold does not cause it to considered large text.

            Original colour values:

            #FF0000 foreground / #FFAAAA background: Failed AA and AAA for normal and large text.

            My proposed solution:

            #A00000 foreground / #FFF0F0 background: 7.6:1 ratio - Passed AA and AAA for normal and large text.

            Here is what you are proposing. I'm not sure how you wanted the #EE0000 to be applied to I tried them all on both the original and my proposed colours:

            #EE0000 foreground / #FFAAAA background: 2.5:1 ratio - Failed AA and AAA for normal and large text.
            #EE0000 foreground / #FFF0F0 background: 4.1:1 ratio - Failed most but passed AA for large text.
            #FF0000 foreground / #EE0000 background: 1.1:1 ratio - Failed AA and AAA for normal and large text.
            #A00000 foreground / #EE0000 background: 1.9:1 ratio - Failed AA and AAA for normal and large text.

            I suspect you may be relying on a tool that does not correctly evaluate colour contrast ratios. The tools I use to perform colour contrast testing are:

            For your convenience, I am including some new alternatives which I have just created if you want to really live right on the edge of approval:

            #A00000 foreground / #FFA5A5 background: 4.5:1 ratio - Passes AA for normal and large text.
            #E20000 foreground / #FFF0F0 background: 4.5:1 ratio - Passes AA for normal and large text.
            #A00000 foreground / #FFE5E5 background: 7.1:1 ratio - Passes AA and AAA for normal and large text.
            #A90000 foreground / #FFF0F0 background: 7.1:1 ratio - Passes AA and AAA for normal and large text.

            For this last one, #AA0000 may or may not pass depending on the algorithm used to evaluate which is why I opted for #A90000 instead. It's that close to the edge!

            As for fixing the colours in all of the other core themes... Themes are plugins in Moodle. Other themes are beyond the scope of the changes requested here and of the work I do for my client. Because I am fixing some accessibility issues with the Feedback plugin doesn't mean I am responsible for fixing all of the other Moodle plugins. Core Moodle HQ developers would be the right people to make such changes.

            Please let me know which colour combination mentioned above meets with your approval and I will be happy to implement it as well as replacing the TABS which inadvertently got inserted into the css style sheets.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - - edited Hi Damyon, Thank you for your feedback. I can see that you've put a lot of thought into this. I will address the colour contrast ratio in this message and follow up with your other concerns in separate comments. The following success criterions are relevant here depending on whether you are aiming for compliance with level AA or AAA. Our client, the Government of Canada requires WCAG 2.0 level AA compliance. SC 1.4.3 Contrast (Minimum): The visual presentation of text and images of text has a contrast ratio of at least 4.5:1, except for the following: (Level AA) Large Print: Large-scale text and images of large-scale text have a contrast ratio of at least 3:1. SC 1.4.6 Contrast (Enhanced): The visual presentation of text and images of text has a contrast ratio of at least 7:1, except for the following: (Level AAA) Large Print: Large-scale text and images of large-scale text have a contrast ratio of at least 4.5:1. It is important to note in this situation that making the text bold does not cause it to considered large text. Original colour values: #FF0000 foreground / #FFAAAA background: Failed AA and AAA for normal and large text. My proposed solution: #A00000 foreground / #FFF0F0 background: 7.6:1 ratio - Passed AA and AAA for normal and large text. Here is what you are proposing. I'm not sure how you wanted the #EE0000 to be applied to I tried them all on both the original and my proposed colours: #EE0000 foreground / #FFAAAA background: 2.5:1 ratio - Failed AA and AAA for normal and large text. #EE0000 foreground / #FFF0F0 background: 4.1:1 ratio - Failed most but passed AA for large text. #FF0000 foreground / #EE0000 background: 1.1:1 ratio - Failed AA and AAA for normal and large text. #A00000 foreground / #EE0000 background: 1.9:1 ratio - Failed AA and AAA for normal and large text. I suspect you may be relying on a tool that does not correctly evaluate colour contrast ratios. The tools I use to perform colour contrast testing are: Online: http://webaim.org/resources/contrastchecker/ Windows: Colour Contrast Analyser v2.2 from http://www.accessibleinfo.org.au/ For your convenience, I am including some new alternatives which I have just created if you want to really live right on the edge of approval: #A00000 foreground / #FFA5A5 background: 4.5:1 ratio - Passes AA for normal and large text. #E20000 foreground / #FFF0F0 background: 4.5:1 ratio - Passes AA for normal and large text. #A00000 foreground / #FFE5E5 background: 7.1:1 ratio - Passes AA and AAA for normal and large text. #A90000 foreground / #FFF0F0 background: 7.1:1 ratio - Passes AA and AAA for normal and large text. For this last one, #AA0000 may or may not pass depending on the algorithm used to evaluate which is why I opted for #A90000 instead. It's that close to the edge! As for fixing the colours in all of the other core themes... Themes are plugins in Moodle. Other themes are beyond the scope of the changes requested here and of the work I do for my client. Because I am fixing some accessibility issues with the Feedback plugin doesn't mean I am responsible for fixing all of the other Moodle plugins. Core Moodle HQ developers would be the right people to make such changes. Please let me know which colour combination mentioned above meets with your approval and I will be happy to implement it as well as replacing the TABS which inadvertently got inserted into the css style sheets. Best regards, Michael
            Hide
            michael-milette Michael Milette added a comment -

            Hi Damyon,

            You said "Our solution for this in normal moodle forms is different - and we should use the same markup everywhere. Ideally all these feedback forms could be moodle forms and then we would have no issue."

            They look like moodleforms to me, are they not?

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - Hi Damyon, You said "Our solution for this in normal moodle forms is different - and we should use the same markup everywhere. Ideally all these feedback forms could be moodle forms and then we would have no issue." They look like moodleforms to me, are they not? Best regards, Michael
            Hide
            michael-milette Michael Milette added a comment -

            Hi Damyon,

            I am hearing you and can understand the desire for consistency. I recreate the patch from scratch taking your suggestions into account. You'll also be happy to hear that I rebased at the same time.

            I literally borrowed code from Moodle forms and the solution seems consistent with the official WCAG examples:
            http://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/H90#H90-examples

            Similar code used in other Moodle forms is now used in Feedback to display the "There are required fields in this form marked (*/Required field)" message at the top of the screen and each of the asterisks next to required fields.

            As for colours, the Feedback module specific styles (missingrequire, feeback_required_mark), which were used to highlight required and error fields, have been eliminated. These colours are now controlled by the common Moodle styles (mform1, mform, fdescription, required, req and error) that are used throughout the rest of Moodle for a more uniform appearance. The colours are not WCAG compliant but that is a common global Moodle issue to be addressed by Moodle HQ.

            Finally, I've updated the Pull Master Diff URL with a new link to the updated changes.

            I've put in a lot of work and testing in making these changes and sincerely hope this meets with your approval.

            Thanks again for your feedback. It's much appreciated.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - Hi Damyon, I am hearing you and can understand the desire for consistency. I recreate the patch from scratch taking your suggestions into account. You'll also be happy to hear that I rebased at the same time. I literally borrowed code from Moodle forms and the solution seems consistent with the official WCAG examples: http://www.w3.org/TR/2013/NOTE-WCAG20-TECHS-20130905/H90#H90-examples Similar code used in other Moodle forms is now used in Feedback to display the "There are required fields in this form marked (*/Required field)" message at the top of the screen and each of the asterisks next to required fields. As for colours, the Feedback module specific styles (missingrequire, feeback_required_mark), which were used to highlight required and error fields, have been eliminated. These colours are now controlled by the common Moodle styles (mform1, mform, fdescription, required, req and error) that are used throughout the rest of Moodle for a more uniform appearance. The colours are not WCAG compliant but that is a common global Moodle issue to be addressed by Moodle HQ. Finally, I've updated the Pull Master Diff URL with a new link to the updated changes. I've put in a lot of work and testing in making these changes and sincerely hope this meets with your approval. Thanks again for your feedback. It's much appreciated. Best regards, Michael
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Error: all the branch fields are incorrect. Nothing was checked.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Michael,

            Looking at this patch, I have a one small suggestion.

            1. id="mform1" - I would leave that out. In a moodleform - this number depends on the number of forms in the page, on a page combining this form and a real moodleform (maybe in a block) this will lead to duplicate ids in the page and possible javascript errors. none of the CSS will rely on the ids (because they can change) so it would be better to not include it.

            I also added the cime tag to this issue to get a code checker report to see if there are any other small things that could be fixed before integration - the cibot will report soon.

            You should update the testing instructions to cover be explicit about how to test with the attached form and what to test/ignore (e.g. the colours may still not meet AA).

            Thanks!

            Show
            damyon Damyon Wiese added a comment - Thanks Michael, Looking at this patch, I have a one small suggestion. 1. id="mform1" - I would leave that out. In a moodleform - this number depends on the number of forms in the page, on a page combining this form and a real moodleform (maybe in a block) this will lead to duplicate ids in the page and possible javascript errors. none of the CSS will rely on the ids (because they can change) so it would be better to not include it. I also added the cime tag to this issue to get a code checker report to see if there are any other small things that could be fixed before integration - the cibot will report soon. You should update the testing instructions to cover be explicit about how to test with the attached form and what to test/ignore (e.g. the colours may still not meet AA). Thanks!
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1035 Warning: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle has not been rebased recently (>20 days ago). Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1035/artifact/work/smurf.html
            Hide
            damyon Damyon Wiese added a comment -

            The cibot comment above contains a link to the smurf.html report of minor coding issues that also need addressing in this patch. Please take a look at these and clean them up too. These are just errors reported for lines you have changed in your patch - it is known that the surrounding code will probably all have the same sorts of errors, but we would like to prevent new code/changes from introducing more errors.

            Thanks!

            Show
            damyon Damyon Wiese added a comment - The cibot comment above contains a link to the smurf.html report of minor coding issues that also need addressing in this patch. Please take a look at these and clean them up too. These are just errors reported for lines you have changed in your patch - it is known that the surrounding code will probably all have the same sorts of errors, but we would like to prevent new code/changes from introducing more errors. Thanks!
            Hide
            michael-milette Michael Milette added a comment -

            Addressed CiBot reported issues. Updated Pull Master Diff.

            Show
            michael-milette Michael Milette added a comment - Addressed CiBot reported issues. Updated Pull Master Diff.
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1071 Warning: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle has not been rebased recently (>20 days ago). Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1071/artifact/work/smurf.html
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1072 Warning: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle has not been rebased recently (>20 days ago). Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1072/artifact/work/smurf.html
            Hide
            michael-milette Michael Milette added a comment -

            Removed language file string that is no longer used:

            Answers are required to starred questions.

            Updated Pull Master Diff URL.

            Show
            michael-milette Michael Milette added a comment - Removed language file string that is no longer used: Answers are required to starred questions. Updated Pull Master Diff URL.
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1073 Warning: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle has not been rebased recently (>20 days ago). Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1073/artifact/work/smurf.html
            Hide
            michael-milette Michael Milette added a comment - - edited

            Code change: Error reporting is now much closer to other Moodle forms. When you don't complete a required field, instead of the field label changing colour, a message is now inserted right below the field label.

            Issue description clarified.

            Testing instructions updated.

            Pull Master Diff URL updated.

            Show
            michael-milette Michael Milette added a comment - - edited Code change: Error reporting is now much closer to other Moodle forms. When you don't complete a required field, instead of the field label changing colour, a message is now inserted right below the field label. Issue description clarified. Testing instructions updated. Pull Master Diff URL updated.
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1079 Warning: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle has not been rebased recently (>20 days ago). Error: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle does not apply clean to master
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1084 Warning: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle has not been rebased recently (>20 days ago). Error: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle does not apply clean to master
            Hide
            michael-milette Michael Milette added a comment -

            Hi Damyon,

            I'm not sure what the problem is. Can you provide some guidance?

            I rebased as per the instructions on the Git for Developers page but it didn't make any difference.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - Hi Damyon, I'm not sure what the problem is. Can you provide some guidance? I rebased as per the instructions on the Git for Developers page but it didn't make any difference. Best regards, Michael
            Hide
            michael-milette Michael Milette added a comment -

            Fixed one last spot that I had missed.

            Show
            michael-milette Michael Milette added a comment - Fixed one last spot that I had missed.
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1270 Warning: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle has not been rebased recently (>20 days ago). Error: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle does not apply clean to master
            Hide
            michael-milette Michael Milette added a comment -

            Rebased and updated Pull Master Diff URL. Not sure what more I should be doing to resolve the issue. Any recommendations would be appreciated.

            Show
            michael-milette Michael Milette added a comment - Rebased and updated Pull Master Diff URL. Not sure what more I should be doing to resolve the issue. Any recommendations would be appreciated.
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1273 Warning: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle has not been rebased recently (>20 days ago). Error: The MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red branch at https://github.com/michael-milette/moodle does not apply clean to master
            Hide
            michael-milette Michael Milette added a comment -

            I'm not sure what more needs to be done about this error. Suggestions anyone... please?

            Show
            michael-milette Michael Milette added a comment - I'm not sure what more needs to be done about this error. Suggestions anyone... please?
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Michael,

            The error means that your dev branch has not been rebased on the latest integration master branch lately, and there are conflicts between the two. Most times the conflicts are easy to resolve.

            git pull --rebase origin master

            Will do the rebase and leaves me with 2 conflicting files:
            git status

            1. both modified: mod/feedback/item/multichoice/lib.php
            2. both modified: mod/feedback/item/multichoicerated/lib.php

            The conflicts look like this:

            <<<<<<< HEAD
                        echo '<label for="'. $item->typ . '_' . $item->id .'">';
                        echo format_text($item->name . $requiredmark, FORMAT_HTML, array('noclean' => true, 'para' => false));
            ||||||| merged common ancestors
                        echo '<label for="'. $item->typ . '_' . $item->id .'">';
                        echo format_text($item->name.$requiredmark, true, false, false);
            =======
                        echo '<label for="'. $inputname .'">';
                        echo format_text($item->name.$requiredmark, true, false, false);
                        if ($highlightrequire AND $item->required AND (count($values) == 0 OR $values[0] == '' OR $values[0] == 0)) {           
                            echo '<br class="error"><span id="id_error_'.$inputname.'" class="error"> '.get_string('err_required', 'form').     
                                '</span><br id="id_error_break_'.$inputname.'" class="error" >';
                        }
            >>>>>>> MDL-42462 - Feedback - Fix for WCAG 2.0 issue with Informational and Error Content Displayed in red
                        echo '</label>';
                    } else {
            <<<<<<< HEAD
                        echo format_text($item->name . $requiredmark, FORMAT_HTML, array('noclean' => true, 'para' => false));
            ||||||| merged common ancestors
                        echo format_text($item->name.$requiredmark, true, false, false);
            =======
                        echo format_text($item->name.$requiredmark, true, false, false);
                        if ($highlightrequire AND $item->required AND (count($values) == 0 OR $values[0] == '' OR $values[0] == 0)) {           
                            echo '<br class="error"><span id="id_error_'.$inputname.'" class="error"> '.get_string('err_required', 'form').     
                                '</span><br id="id_error_break_'.$inputname.'" class="error" >';
                        }
            >>>>>>> MDL-42462 - Feedback - Fix for WCAG 2.0 issue with Informational and Error Content Displayed in red
             

            Which shows the upstream version, the current master version - and your modified version. Looking at that conflict makes me wonder why you changed the for attribute on the label. It must point to the id of the form element that it describes or screen readers will not read the label for the form field.

            Can you please do this rebase and comment about the "for" change? This is almost ready to send for integration once we get over these last hurdles.

            Thanks!

            Show
            damyon Damyon Wiese added a comment - Thanks Michael, The error means that your dev branch has not been rebased on the latest integration master branch lately, and there are conflicts between the two. Most times the conflicts are easy to resolve. git pull --rebase origin master Will do the rebase and leaves me with 2 conflicting files: git status both modified: mod/feedback/item/multichoice/lib.php both modified: mod/feedback/item/multichoicerated/lib.php The conflicts look like this: <<<<<<< HEAD echo '<label for="'. $item->typ . '_' . $item->id .'">'; echo format_text($item->name . $requiredmark, FORMAT_HTML, array('noclean' => true, 'para' => false)); ||||||| merged common ancestors echo '<label for="'. $item->typ . '_' . $item->id .'">'; echo format_text($item->name.$requiredmark, true, false, false); ======= echo '<label for="'. $inputname .'">'; echo format_text($item->name.$requiredmark, true, false, false); if ($highlightrequire AND $item->required AND (count($values) == 0 OR $values[0] == '' OR $values[0] == 0)) { echo '<br class="error"><span id="id_error_'.$inputname.'" class="error"> '.get_string('err_required', 'form'). '</span><br id="id_error_break_'.$inputname.'" class="error" >'; } >>>>>>> MDL-42462 - Feedback - Fix for WCAG 2.0 issue with Informational and Error Content Displayed in red echo '</label>'; } else { <<<<<<< HEAD echo format_text($item->name . $requiredmark, FORMAT_HTML, array('noclean' => true, 'para' => false)); ||||||| merged common ancestors echo format_text($item->name.$requiredmark, true, false, false); ======= echo format_text($item->name.$requiredmark, true, false, false); if ($highlightrequire AND $item->required AND (count($values) == 0 OR $values[0] == '' OR $values[0] == 0)) { echo '<br class="error"><span id="id_error_'.$inputname.'" class="error"> '.get_string('err_required', 'form'). '</span><br id="id_error_break_'.$inputname.'" class="error" >'; } >>>>>>> MDL-42462 - Feedback - Fix for WCAG 2.0 issue with Informational and Error Content Displayed in red   Which shows the upstream version, the current master version - and your modified version. Looking at that conflict makes me wonder why you changed the for attribute on the label. It must point to the id of the form element that it describes or screen readers will not read the label for the form field. Can you please do this rebase and comment about the "for" change? This is almost ready to send for integration once we get over these last hurdles. Thanks!
            Hide
            michael-milette Michael Milette added a comment -

            I've never had much luck with rebasing. There is obviously some step that I must be missing because it just doesn't work for me. Here is what I do to rebase. I got these instructions from the Git for Developers page at http://docs.moodle.org/dev/Git_for_developers. Maybe you can spot where I am going wrong:

            First I sync my GitHub fork with the official Moodle repository:

            $ git checkout master
            $ git fetch upstream
            $ for BRANCH in MOODLE_24_STABLE MOODLE_25_STABLE MOODLE_26_STABLE master; do git push origin refs/remotes/upstream/$BRANCH:$BRANCH;done;

            Then I rebase each of my branches:

            $ git rebase master MDL-XXXXX-description

            I did this several times today trying to resolve the issue but it never seemed to make any difference.

            I also tried the two commands you mentioned in your message:

            $ git pull --rebase origin master
            From github.com:michael-milette/moodle
             * branch            master     -> FETCH_HEAD
            First, rewinding head to replay your work on top of it...
            Fast-forwarded master to 974c2cdc03caeb36ba714b9db551fb6c9e9303de.
             
            $ git status
            # On branch master
            nothing to commit, working directory clean

            So when that didn't seem to make any difference, I tried this:

            $ git checkout MDL-42462-Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red
            Checking out files: 100% (1084/1084), done.
            Switched to branch 'MDL-42462-Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red'
             
            $ git status
            # On branch MDL-42462-Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red
            nothing to commit, working directory clean

            If you can spot what is wrong or if I am missing some step, I would be very grateful if you would let me know as it would save me and those I work with in the Moodle community a lot of "rebasing" frustrations with this and many other patches I've written.

            When all else fails, my strategy has been to wipe my local copy and the online copy completely and start over but this takes a lot of time and I know there has to be a better solution using the features in Git.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - I've never had much luck with rebasing. There is obviously some step that I must be missing because it just doesn't work for me. Here is what I do to rebase. I got these instructions from the Git for Developers page at http://docs.moodle.org/dev/Git_for_developers . Maybe you can spot where I am going wrong: First I sync my GitHub fork with the official Moodle repository: $ git checkout master $ git fetch upstream $ for BRANCH in MOODLE_24_STABLE MOODLE_25_STABLE MOODLE_26_STABLE master; do git push origin refs/remotes/upstream/$BRANCH:$BRANCH;done; Then I rebase each of my branches: $ git rebase master MDL-XXXXX-description I did this several times today trying to resolve the issue but it never seemed to make any difference. I also tried the two commands you mentioned in your message: $ git pull --rebase origin master From github.com:michael-milette/moodle * branch master -> FETCH_HEAD First, rewinding head to replay your work on top of it... Fast-forwarded master to 974c2cdc03caeb36ba714b9db551fb6c9e9303de.   $ git status # On branch master nothing to commit, working directory clean So when that didn't seem to make any difference, I tried this: $ git checkout MDL-42462-Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red Checking out files: 100% (1084/1084), done. Switched to branch 'MDL-42462-Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red'   $ git status # On branch MDL-42462-Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red nothing to commit, working directory clean If you can spot what is wrong or if I am missing some step, I would be very grateful if you would let me know as it would save me and those I work with in the Moodle community a lot of "rebasing" frustrations with this and many other patches I've written. When all else fails, my strategy has been to wipe my local copy and the online copy completely and start over but this takes a lot of time and I know there has to be a better solution using the features in Git. Best regards, Michael
            Hide
            bawjaws David Scotson added a comment -

            The linked diff has code a bit like this roughly 20 times (and it's possible there's more in Moodle already):

            echo get_string('somefieldsrequired', 'form', '<img alt="'.get_string('requiredelement', 'form').'" src="'.$OUTPUT->pix_url('req') .'" class="req" />');

            It might be nice to have this in one centrally located function and then call it from the other 19, particularly as there were small differences in order of the attributes that confused me into thinking they were two different variations, though as far as the browser is concerned they're identical.

            The other change seems repeated a few times too and could similarly be a function perhaps?

            Show
            bawjaws David Scotson added a comment - The linked diff has code a bit like this roughly 20 times (and it's possible there's more in Moodle already): echo get_string('somefieldsrequired', 'form', '<img alt="'.get_string('requiredelement', 'form').'" src="'.$OUTPUT->pix_url('req') .'" class="req" />'); It might be nice to have this in one centrally located function and then call it from the other 19, particularly as there were small differences in order of the attributes that confused me into thinking they were two different variations, though as far as the browser is concerned they're identical. The other change seems repeated a few times too and could similarly be a function perhaps?
            Hide
            michael-milette Michael Milette added a comment -

            I think I figured out the rebase. I've updated the Pull Master Diff URL. Let's see if CiBot agrees.

            Show
            michael-milette Michael Milette added a comment - I think I figured out the rebase. I've updated the Pull Master Diff URL. Let's see if CiBot agrees.
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1460 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1460/artifact/work/smurf.html
            Hide
            michael-milette Michael Milette added a comment -

            That should do it!

            Show
            michael-milette Michael Milette added a comment - That should do it!
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1513 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1513/artifact/work/smurf.html
            Hide
            michael-milette Michael Milette added a comment -

            Please let me know if there is anything I can do to help move this patch forward.

            Show
            michael-milette Michael Milette added a comment - Please let me know if there is anything I can do to help move this patch forward.
            Hide
            michael-milette Michael Milette added a comment -

            Rebased and updated the Pull Master Diff URL field.

            Show
            michael-milette Michael Milette added a comment - Rebased and updated the Pull Master Diff URL field.
            Hide
            michael-milette Michael Milette added a comment -

            Is there anything I can do to help move this issue forward?

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - Is there anything I can do to help move this issue forward? Best regards, Michael
            Hide
            michael-milette Michael Milette added a comment -

            For your convenience, I've rebased the code and updated the Pull Master Diff URL. No other changes were made.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - For your convenience, I've rebased the code and updated the Pull Master Diff URL. No other changes were made. Best regards, Michael
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/2085 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/2085/artifact/work/smurf.html
            Hide
            michael-milette Michael Milette added a comment -

            Rebased it today just in case someone would like to review it.

            The last message from Damyon seemed to indicate that it was ready and I just had to rebase it. That was over a month ago.

            Any chance of getting this integrated in time for the Moodle 2.7 release?

            Note: As for backporting it, unfortunately I have been unable to get that working. Still getting messages from Cime that my backports are 60+ days too old when I create them.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - Rebased it today just in case someone would like to review it. The last message from Damyon seemed to indicate that it was ready and I just had to rebase it. That was over a month ago. Any chance of getting this integrated in time for the Moodle 2.7 release? Note: As for backporting it, unfortunately I have been unable to get that working. Still getting messages from Cime that my backports are 60+ days too old when I create them. Best regards, Michael
            Hide
            cibot CiBoT added a comment -

            Results for MDL-42462

            Show
            cibot CiBoT added a comment - Results for MDL-42462 Remote repository: https://github.com/michael-milette/moodle Remote branch MDL-42462 -Feedback-Fix-for-WCAG-2-0-issue-with-Informational-and-Error-Content-Displayed-in-red to be integrated into upstream master Executed job http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2410 Details: http://social.srv.in.moodle.com/job/Precheck%20remote%20branch/2410/artifact/work/smurf.html
            Hide
            michael-milette Michael Milette added a comment -

            Bump

            Show
            michael-milette Michael Milette added a comment - Bump
            Hide
            damyon Damyon Wiese added a comment -

            Sorry for the delay Michael, the rebase looks good and this looks ready to me. I noticed a tiny bug when testing this again - but it's not caused by this issue and I'll add a follow up issue to fix it.

            Sending for integration.

            Show
            damyon Damyon Wiese added a comment - Sorry for the delay Michael, the rebase looks good and this looks ready to me. I noticed a tiny bug when testing this again - but it's not caused by this issue and I'll add a follow up issue to fix it. Sending for integration.
            Hide
            damyon Damyon Wiese added a comment -

            MDL-44952 is the issue I found.

            Show
            damyon Damyon Wiese added a comment - MDL-44952 is the issue I found.
            Hide
            cibot CiBoT added a comment -

            Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

            Show
            cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
            Hide
            poltawski Dan Poltawski added a comment -

            Thankyou Michael, i've integrated this to master, 26 and 25 (cherry-pikced to 25/26)

            Show
            poltawski Dan Poltawski added a comment - Thankyou Michael, i've integrated this to master, 26 and 25 (cherry-pikced to 25/26)
            Hide
            markn Mark Nelson added a comment -

            This issue introduces the use of $OUTPUT->pix_url in the function print_item_preview() in mod/feedback/item/captcha/lib.php but doesn't declare the $OUTPUT global variable which causes the following error - Notice: Undefined variable: OUTPUT in /Users/mark/htdocs/mstorage/i26/moodle/mod/feedback/item/captcha/lib.php on line 138.

            If you could push the following commit into integration to solve this issue that would be great, then I will continue testing.

            https://github.com/markn86/moodle/commit/fabdf9b33731e06e25ceee650f851a257f370be1

            Show
            markn Mark Nelson added a comment - This issue introduces the use of $OUTPUT->pix_url in the function print_item_preview() in mod/feedback/item/captcha/lib.php but doesn't declare the $OUTPUT global variable which causes the following error - Notice: Undefined variable: OUTPUT in /Users/mark/htdocs/mstorage/i26/moodle/mod/feedback/item/captcha/lib.php on line 138. If you could push the following commit into integration to solve this issue that would be great, then I will continue testing. https://github.com/markn86/moodle/commit/fabdf9b33731e06e25ceee650f851a257f370be1
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks for the fix Mark, pulled

            Show
            poltawski Dan Poltawski added a comment - Thanks for the fix Mark, pulled
            Hide
            markn Mark Nelson added a comment -

            Thanks Dan. I am going to bed now (it is 1am here) and I have to get up early to view a house to move into for Summer over here. I will test this tomorrow (probably before you guys get into the office in HQ).

            Cheers!

            Show
            markn Mark Nelson added a comment - Thanks Dan. I am going to bed now (it is 1am here) and I have to get up early to view a house to move into for Summer over here. I will test this tomorrow (probably before you guys get into the office in HQ). Cheers!
            Hide
            markn Mark Nelson added a comment -

            Hi Michael,

            I am failing this due to step number 2. The text "Saving failed because missing or false values" is not the same format as "There are required fields in this form marked". The text is actually the same as "You must supply a value here." and is also displayed in a red box.

            Thanks.

            Show
            markn Mark Nelson added a comment - Hi Michael, I am failing this due to step number 2. The text "Saving failed because missing or false values" is not the same format as "There are required fields in this form marked". The text is actually the same as "You must supply a value here." and is also displayed in a red box. Thanks.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Mark - after discussing it with you I think the testing instructions are misleading and in fact the behaviour sounds correct - so i'm passing this.

            Show
            poltawski Dan Poltawski added a comment - Thanks Mark - after discussing it with you I think the testing instructions are misleading and in fact the behaviour sounds correct - so i'm passing this.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks for your efforts, this change is now part of Moodle!

            Show
            poltawski Dan Poltawski added a comment - Thanks for your efforts, this change is now part of Moodle!
            Hide
            michael-milette Michael Milette added a comment -

            Thank you to all who made the integration of this patch possible and not holding it up any longer.

            Appreciation goes out to Dan Poltawski, Mark Nelson, Damyon Wiese, David Scotson and Michael de Raadt.

            Best regards,

            Michael

            Show
            michael-milette Michael Milette added a comment - Thank you to all who made the integration of this patch possible and not holding it up any longer. Appreciation goes out to Dan Poltawski, Mark Nelson, Damyon Wiese, David Scotson and Michael de Raadt. Best regards, Michael

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/May/14