Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-35816 Accessibility Review issues (Deque)
  3. MDL-35847

Login error message text presented in red only. An icon is also needed.

    Details

    • Testing Instructions:
      Hide
      1. Go to the login page
      2. Enter a username and no password
      3. Click the login button
      4. See the icon appear in the error message
      5. Inspect the icon in a webdev tool like firebug
      6. Make sure the title attrib is absent
      7. Make sure the alt attrib contains "Error" (if using English as your language)
      Show
      Go to the login page Enter a username and no password Click the login button See the icon appear in the error message Inspect the icon in a webdev tool like firebug Make sure the title attrib is absent Make sure the alt attrib contains "Error" (if using English as your language)
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-35847-master

      Description

      Issue
      Sensory characteristics - Error messaging for logging in is presented in red only. Some users will not be able to perceive this error message as such based only on the text color. Message should also be supported through another means (ex. An icon before the text).

      Color use - Error messaging for logging in is presented in red only. Some users will not be able to perceive this error message as such based only on the text color. Message should also be supported through another means (ex. An icon before the text).

      Standard Level
      WCAG 2 1.3.3 (A) http://www.w3.org/WAI/WCAG20/quickref/#qr-content-structure-separation-understanding
      WCAG 2 1.4.1 (A) http://www.w3.org/WAI/WCAG20/quickref/#qr-visual-audio-contrast-without-color

      Impact
      Serious

      Example Link
      http://demo.moodle.net/

      Test Steps

      1. Enter teacher username in the user name field
      2. Enter incorrect password
      3. Click the login button
      4. Error message does not say error or have an icon that has an alt text error so a user only has the red color to know it is an error.

        Gliffy Diagrams

        1. warning.svg
          0.9 kB
          Barbara Ramiro
        1. warning.png
          0.3 kB

          Activity

          Hide
          salvetore Michael de Raadt added a comment -

          I mentioned this on another issue. Perhaps we should add an optional image with the notification function.

          Show
          salvetore Michael de Raadt added a comment - I mentioned this on another issue. Perhaps we should add an optional image with the notification function.
          Hide
          barbararamiro Barbara Ramiro added a comment -

          Jason, attached warning.png and warning.svg icons in gray. Should be in pix/i. Please consult Fred for the icon class he used for all the new icons for consistency.

          Cheers

          Show
          barbararamiro Barbara Ramiro added a comment - Jason, attached warning.png and warning.svg icons in gray. Should be in pix/i. Please consult Fred for the icon class he used for all the new icons for consistency. Cheers
          Hide
          phalacee Jason Fowler added a comment -

          Thanks Barbara, wonderful work as always

          Show
          phalacee Jason Fowler added a comment - Thanks Barbara, wonderful work as always
          Hide
          phalacee Jason Fowler added a comment -

          Submitted as two commits, one for Barbara's icons, one for the code changes.

          Show
          phalacee Jason Fowler added a comment - Submitted as two commits, one for Barbara's icons, one for the code changes.
          Hide
          barbararamiro Barbara Ramiro added a comment -

          Thanks Jason =)

          Show
          barbararamiro Barbara Ramiro added a comment - Thanks Jason =)
          Hide
          fred Frédéric Massart added a comment - - edited

          Hi Jason, here are my comments:

          1. Should we really add a new parameter $icon to this function? I know that the purpose of this issue is to add an icon to the login page, but to me that would beneficial for any other error message. Also, wouldn't we want the renderer to take care of whether an icon is present or not? With this option I'm scared that developers would inconsistently use the icon or not. And if the parameter is needed, perhaps setting its default value to true is ideal. What are your thoughts?
          2. You should translate the alt attribute of the icon. It should also start with a capital letter.
          3. I don't think you should overwrite the title for it to be empty. I understand you don't want to duplicate it, but this is an issue coming from pix_icon itself and we should probably fix it there rather than manually here and there.
          4. I think the commit message for Barbara's should not be linked to the login issue. This icon will be generic and used anywhere we need it. As the icon will be generic I'd use 'Usability' as component which is the one we used for 2.4 icons.
          5. Could you comment on the reason why you didn't branch for 2.3 (I understand, just leave a message for the integration team).
          6. You're missing some testing instructions.

          [Y] Syntax
          [Y] Output
          [Y] Whitespace
          [N] Language
          [-] Databases
          [N] Testing
          [-] Security
          [-] Documentation
          [?] Git
          [Y] Sanity check

          Cheers!
          Fred

          Show
          fred Frédéric Massart added a comment - - edited Hi Jason, here are my comments: Should we really add a new parameter $icon to this function? I know that the purpose of this issue is to add an icon to the login page, but to me that would beneficial for any other error message. Also, wouldn't we want the renderer to take care of whether an icon is present or not? With this option I'm scared that developers would inconsistently use the icon or not. And if the parameter is needed, perhaps setting its default value to true is ideal. What are your thoughts? You should translate the alt attribute of the icon. It should also start with a capital letter. I don't think you should overwrite the title for it to be empty. I understand you don't want to duplicate it, but this is an issue coming from pix_icon itself and we should probably fix it there rather than manually here and there. I think the commit message for Barbara's should not be linked to the login issue. This icon will be generic and used anywhere we need it. As the icon will be generic I'd use 'Usability' as component which is the one we used for 2.4 icons. Could you comment on the reason why you didn't branch for 2.3 (I understand, just leave a message for the integration team). You're missing some testing instructions. [Y] Syntax [Y] Output [Y] Whitespace [N] Language [-] Databases [N] Testing [-] Security [-] Documentation [?] Git [Y] Sanity check Cheers! Fred
          Hide
          phalacee Jason Fowler added a comment -
          1. The param is there to make it optional, there are a few places this function is used, and it may not always be ideal to have the icon there. Much more research would need to be done before it can be made the default setting.
          2. Forgot that, yes, a string would be needed
          3. The title isn't needed. Fixing the pix_icon problem is a different issue altogether, and should be handled as such.
          4. What should I put for the icon issue on Barbara's commit then? The icons were created as part of this issue. They belong here.
          5. I didn't branch for 2.3 because of the icon changes that are only present in 2.4+
          6. Will write them as soon as I have finished the string for the ALT
          Show
          phalacee Jason Fowler added a comment - The param is there to make it optional, there are a few places this function is used, and it may not always be ideal to have the icon there. Much more research would need to be done before it can be made the default setting. Forgot that, yes, a string would be needed The title isn't needed. Fixing the pix_icon problem is a different issue altogether, and should be handled as such. What should I put for the icon issue on Barbara's commit then? The icons were created as part of this issue. They belong here. I didn't branch for 2.3 because of the icon changes that are only present in 2.4+ Will write them as soon as I have finished the string for the ALT
          Hide
          phalacee Jason Fowler added a comment -

          Changed the error alt text to a get_string call

          Show
          phalacee Jason Fowler added a comment - Changed the error alt text to a get_string call
          Hide
          fred Frédéric Massart added a comment -

          1/ I understand but creating the parameter for the only purpose of using it on the login page (for now) does not really convince me, however it's up to you.
          3/ I know it should be handled in a different issue, I'm just saying that you should maybe not use a workaround.
          4/ MDL-35847 usability: New warning icon

          Show
          fred Frédéric Massart added a comment - 1/ I understand but creating the parameter for the only purpose of using it on the login page (for now) does not really convince me, however it's up to you. 3/ I know it should be handled in a different issue, I'm just saying that you should maybe not use a workaround. 4/ MDL-35847 usability: New warning icon
          Hide
          damyon Damyon Wiese added a comment -

          -1 for the icon parameter. This usability issue would affect everywhere error_text is called so they should all get the icon.

          Show
          damyon Damyon Wiese added a comment - -1 for the icon parameter. This usability issue would affect everywhere error_text is called so they should all get the icon.
          Hide
          phalacee Jason Fowler added a comment - - edited

          Okay, I'll remove the icon, as Damyon and Fred have advised, and make this a master only change then.

          Show
          phalacee Jason Fowler added a comment - - edited Okay, I'll remove the icon, as Damyon and Fred have advised, and make this a master only change then.
          Hide
          poltawski Dan Poltawski added a comment -

          Yep, I also agree. Reopening for that. Thanks Jason

          Show
          poltawski Dan Poltawski added a comment - Yep, I also agree. Reopening for that. Thanks Jason
          Hide
          cibot CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          phalacee Jason Fowler added a comment -

          Done.

          Show
          phalacee Jason Fowler added a comment - Done.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Integrated (master only), thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Integrated (master only), thanks!
          Hide
          phalacee Jason Fowler added a comment -

          Eloy! the param for $icon that was removed, but I left the doc block in for it, I have now updated it with a new commit.

          Show
          phalacee Jason Fowler added a comment - Eloy! the param for $icon that was removed, but I left the doc block in for it, I have now updated it with a new commit.
          Hide
          poltawski Dan Poltawski added a comment -

          Pulled that fix in.

          Show
          poltawski Dan Poltawski added a comment - Pulled that fix in.
          Hide
          rwijaya Rossiani Wijaya added a comment -

          This is working as expected.

          Test it for master only.

          Test passed.

          Show
          rwijaya Rossiani Wijaya added a comment - This is working as expected. Test it for master only. Test passed.
          Hide
          damyon Damyon Wiese added a comment -

          Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone!

          Regards, Damyon

          Show
          damyon Damyon Wiese added a comment - Congratulations! This issue has been resolved. Thanks for helping to make Moodle better for everyone! Regards, Damyon

            People

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

              Dates

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