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
            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