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

Add activity or resource plus icon has same alt text as span

    Details

    • Testing Instructions:
      Hide
      1. login as admin
      2. turn on editing
      3. inspect the code for the "+ Add activity or resource" link
      4. Ensure the alt attribute is empty
      5. With a screen reader tab to the add activity or resource link and ensure item not repeated
      Show
      login as admin turn on editing inspect the code for the "+ Add activity or resource" link Ensure the alt attribute is empty With a screen reader tab to the add activity or resource link and ensure item not repeated
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-35884-master

      Description

      Issue
      Alt text - The plus icon to the left of the Add activity or resource link when editing is turned on for a course has the same text as the link. These should have an empty alt text instead

      Standard Level
      WCAG 2 1.1.1 (A) http://www.w3.org/WAI/WCAG20/quickref/#qr-text-equiv-all

      Impact
      Serious

      Example Link
      http://demo.moodle.net/course/view.php?id=625&notifyeditingon=1

      Test Steps

      1. Login as a teacher
      2. navigate to the demo science course
      3. Turn editing on
      4. With a screen reader tab to the add activity or resource link

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              salvetore Michael de Raadt added a comment -

              Perhaps for complete correctness, the title on the link and the alt on the image should not be the same, but the most appropriate value.

              See http://docs.moodle.org/dev/Accessibility#Moodle-related_accessibility_coding_guidelines

              Show
              salvetore Michael de Raadt added a comment - Perhaps for complete correctness, the title on the link and the alt on the image should not be the same, but the most appropriate value. See http://docs.moodle.org/dev/Accessibility#Moodle-related_accessibility_coding_guidelines
              Hide
              phalacee Jason Fowler added a comment -

              requesting peer review before I cherry-pick to stable branches

              Show
              phalacee Jason Fowler added a comment - requesting peer review before I cherry-pick to stable branches
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Patch looks good Jason,
              Personally I would have left component = null, as it should take default value from the api (Original case).

              Feel free to push it for integration.
              [y] Syntax
              [y] Output
              [y] Whitespace
              [-] Language
              [-] Databases
              [y] Testing
              [-] Security
              [-] Documentation
              [y] Git
              [y] Sanity check

              Show
              rajeshtaneja Rajesh Taneja added a comment - Patch looks good Jason, Personally I would have left component = null, as it should take default value from the api (Original case). Feel free to push it for integration. [y] Syntax [y] Output [y] Whitespace [-] Language [-] Databases [y] Testing [-] Security [-] Documentation [y] Git [y] Sanity check
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Looking at this again, just wondering if we really need title for this?
              Don't you think + is only for display purpose, why would a user need title popup with same string as it appears next to it.

              Show
              rajeshtaneja Rajesh Taneja added a comment - Looking at this again, just wondering if we really need title for this? Don't you think + is only for display purpose, why would a user need title popup with same string as it appears next to it.
              Hide
              phalacee Jason Fowler added a comment -

              Thanks for pointing that out Raj, will remove the title as well.

              Show
              phalacee Jason Fowler added a comment - Thanks for pointing that out Raj, will remove the title as well.
              Hide
              phalacee Jason Fowler added a comment -

              Changes made, going for integration now

              Show
              phalacee Jason Fowler added a comment - Changes made, going for integration now
              Hide
              poltawski Dan Poltawski added a comment -
              Show
              poltawski Dan Poltawski added a comment - Related dicussion to this: https://moodle.org/mod/forum/discuss.php?d=197963
              Hide
              poltawski Dan Poltawski added a comment -

              I have some reservations about this, mostly because I am not an expert in this area. Is it really better for us to use these empty strings?

              If so, should the pix_icon renderer really be '@param string $alt mandatory alt attribute'? I'd like some more opinions on this, so asking around.

              Show
              poltawski Dan Poltawski added a comment - I have some reservations about this, mostly because I am not an expert in this area. Is it really better for us to use these empty strings? If so, should the pix_icon renderer really be '@param string $alt mandatory alt attribute'? I'd like some more opinions on this, so asking around.
              Hide
              phalacee Jason Fowler added a comment -

              I'm not sure about the pix_icon alt attribute being mandatory, but the empty strings are correct in this case.

              http://www.un.org/webaccessibility/1_visual/11_alternativetext.shtml explains:

              If an image does not convey any information and only serves a decorative purpose, add an empty "alt" attribute. Decorative elements can be defined in a CSS style sheet.

              Show
              phalacee Jason Fowler added a comment - I'm not sure about the pix_icon alt attribute being mandatory, but the empty strings are correct in this case. http://www.un.org/webaccessibility/1_visual/11_alternativetext.shtml explains: If an image does not convey any information and only serves a decorative purpose, add an empty "alt" attribute. Decorative elements can be defined in a CSS style sheet.
              Hide
              poltawski Dan Poltawski added a comment -

              Hi Jason,

              Thanks, that link is useful. I discussed with the integrators and confirmed that your approach is the correct one.

              However, i'm afraid there is an unrelated line change on the 23 branch, so reopening this for that to be fixed up:
              https://github.com/phalacee/moodle/compare/MOODLE_23_STABLE...wip-MDL-35884-s23

              Show
              poltawski Dan Poltawski added a comment - Hi Jason, Thanks, that link is useful. I discussed with the integrators and confirmed that your approach is the correct one. However, i'm afraid there is an unrelated line change on the 23 branch, so reopening this for that to be fixed up: https://github.com/phalacee/moodle/compare/MOODLE_23_STABLE...wip-MDL-35884-s23
              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 -

              Made the fix as requested Dan - two commits, one to fix the issue, the second to fix the whitespace/character return problem.

              Pushing for integration again

              Show
              phalacee Jason Fowler added a comment - Made the fix as requested Dan - two commits, one to fix the issue, the second to fix the whitespace/character return problem. Pushing for integration again
              Hide
              poltawski Dan Poltawski added a comment -

              Thanks Jason, i've integrated this now to master, 24 and 23.

              Show
              poltawski Dan Poltawski added a comment - Thanks Jason, i've integrated this now to master, 24 and 23.
              Hide
              abgreeve Adrian Greeve added a comment -

              Tested on the 2.3 and master integration branches.
              The alt and title strings are now empty.
              No problems found.
              Test passed.

              Show
              abgreeve Adrian Greeve added a comment - Tested on the 2.3 and master integration branches. The alt and title strings are now empty. No problems found. Test passed.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

              Closing, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

                People

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

                  Dates

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