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

          Issue Links

            Activity

            Hide
            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
            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
            Jason Fowler added a comment -

            requesting peer review before I cherry-pick to stable branches

            Show
            Jason Fowler added a comment - requesting peer review before I cherry-pick to stable branches
            Hide
            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
            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
            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
            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
            Jason Fowler added a comment -

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

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

            Changes made, going for integration now

            Show
            Jason Fowler added a comment - Changes made, going for integration now
            Hide
            Dan Poltawski added a comment -
            Show
            Dan Poltawski added a comment - Related dicussion to this: https://moodle.org/mod/forum/discuss.php?d=197963
            Hide
            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
            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
            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
            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
            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
            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 added a comment -

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

            Show
            CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            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
            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
            Dan Poltawski added a comment -

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

            Show
            Dan Poltawski added a comment - Thanks Jason, i've integrated this now to master, 24 and 23.
            Hide
            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
            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
            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
            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: