Details

    • Testing Instructions:
      Hide

      Test 1 (For master and stables)

      1. This needs to be tested in all supported browsers
      2. Goto site pages>calendar
      3. Try to traverse using a keyboard.
      4. Create a few different type of events if you already dont have any
      5. make sure the mini calendar on the sidebar displays the popup with event details "on focus" (using keyboard) and "on mouseover" (using mouse)
      6. Delete any events corresponding to current date
      7. Make sure the mini calendar on the sidebar displays the popup with content saying "No events" "on focus" (using keyboard) and "on mouseover" (using mouse)

      Test 2 (Just for master)

      1. Make sure for the step 5 and step 7 cases the popup div has "aria-live=off" attribute. Make sure that changes to "aria-live=assertive" when the popup is displayed and changes back to "off" once the popup is invisible.
      2. Make sure all links in the mini calendar on the side bar (corrosponding to days) have "aria-controls='.$popupid.'_panel" atrribute
      Show
      Test 1 (For master and stables) This needs to be tested in all supported browsers Goto site pages>calendar Try to traverse using a keyboard. Create a few different type of events if you already dont have any make sure the mini calendar on the sidebar displays the popup with event details "on focus" (using keyboard) and "on mouseover" (using mouse) Delete any events corresponding to current date Make sure the mini calendar on the sidebar displays the popup with content saying "No events" "on focus" (using keyboard) and "on mouseover" (using mouse) Test 2 (Just for master) Make sure for the step 5 and step 7 cases the popup div has "aria-live=off" attribute. Make sure that changes to "aria-live=assertive" when the popup is displayed and changes back to "off" once the popup is invisible. Make sure all links in the mini calendar on the side bar (corrosponding to days) have "aria-controls='.$popupid.'_panel" atrribute
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-30889-master

      Description

      For the small calendars on the side of the page, either when viewing the main monthly calendar or in the calendar block, events can be seen by hovering over a date with a mouse, but this is not keyboard accessible and is not read by screen readers. An onfocus event, in addition to the onhover event, needs to be added so keyboard events will trigger the popup box. Also the popup will need to have the appropriate ARIA attributes.

        Gliffy Diagrams

          Issue Links

            Activity

            Greg Kraus created issue -
            Greg Kraus made changes -
            Field Original Value New Value
            Fix Version/s 2.3 [ 10657 ]
            Greg Kraus made changes -
            Component/s Accessibility [ 10083 ]
            Michael de Raadt made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Fix Version/s 2.3 [ 10657 ]
            Labels triaged
            Michael de Raadt made changes -
            Link This issue is duplicated by MDL-32122 [ MDL-32122 ]
            Ankit Agarwal made changes -
            Fix Version/s STABLE Sprint 24 Omega [ 12364 ]
            Ankit Agarwal made changes -
            Assignee moodle.com [ moodle.com ] Ankit Agarwal [ ankit_frenz ]
            Ankit Agarwal made changes -
            Fix Version/s STABLE Sprint 24 Alpha [ 12363 ]
            Fix Version/s STABLE backlog [ 10463 ]
            Fix Version/s STABLE Sprint 24 Omega [ 12364 ]
            Ankit Agarwal made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            Ankit Agarwal made changes -
            Hide
            Ankit Agarwal added a comment - - edited

            Am not sure if this is the best way to apply those attributes.
            Requesting a review.

            Will port the first commit to stable and update the testing instruction after the review.
            Thanks

            Show
            Ankit Agarwal added a comment - - edited Am not sure if this is the best way to apply those attributes. Requesting a review. Will port the first commit to stable and update the testing instruction after the review. Thanks
            Ankit Agarwal made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            Ankit Agarwal made changes -
            Labels triaged api_change triaged
            Frédéric Massart made changes -
            Original Estimate 0 minutes [ 0 ]
            Remaining Estimate 0 minutes [ 0 ]
            Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
            Peer reviewer fred
            Hide
            Frédéric Massart added a comment -

            Hi Ankit, the code looks good and is working as expected, I just noticed that the HTML properties introduced 'id' and 'aria-controls' are not escaped with quotes. I would suggest to modify those lines so that they are using html_writer which will solve this problem.

            Cheers!

            Show
            Frédéric Massart added a comment - Hi Ankit, the code looks good and is working as expected, I just noticed that the HTML properties introduced 'id' and 'aria-controls' are not escaped with quotes. I would suggest to modify those lines so that they are using html_writer which will solve this problem. Cheers!
            Frédéric Massart made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            Ankit Agarwal made changes -
            Testing Instructions *Test 1*
            # This needs to be tested in all supported browsers
            # Goto site pages>calendar
            # Try to traverse using a keyboard.
            # Create a few different type of events if you already dont have any
            # make sure the mini calendar on the sidebar displays the popup with event details "on focus" (using keyboard) and "on mouseover" (using mouse)
            # Delete any events corresponding to current date
            # Make sure the mini calendar on the sidebar displays the popup with content saying "No events" "on focus" (using keyboard) and "on mouseover" (using mouse)

            *Test 2*
            # Make sure for the step 5 and step 7 cases the popup div has "aria-live=off" attribute. Make sure that changes to "aria-live=assertive" when the popup is displayed and changes back to "off" once the popup is invisible.
            # Make sure all links in the mini calendar on the side bar (corrosponding to days) have "aria-controls='.$popupid.'_panel" atrribute
            Hide
            Ankit Agarwal added a comment -

            Changes made. Requesting another round of review.
            Thanks

            Show
            Ankit Agarwal added a comment - Changes made. Requesting another round of review. Thanks
            Ankit Agarwal made changes -
            Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
            Ankit Agarwal made changes -
            Testing Instructions *Test 1*
            # This needs to be tested in all supported browsers
            # Goto site pages>calendar
            # Try to traverse using a keyboard.
            # Create a few different type of events if you already dont have any
            # make sure the mini calendar on the sidebar displays the popup with event details "on focus" (using keyboard) and "on mouseover" (using mouse)
            # Delete any events corresponding to current date
            # Make sure the mini calendar on the sidebar displays the popup with content saying "No events" "on focus" (using keyboard) and "on mouseover" (using mouse)

            *Test 2*
            # Make sure for the step 5 and step 7 cases the popup div has "aria-live=off" attribute. Make sure that changes to "aria-live=assertive" when the popup is displayed and changes back to "off" once the popup is invisible.
            # Make sure all links in the mini calendar on the side bar (corrosponding to days) have "aria-controls='.$popupid.'_panel" atrribute
            *Test 1 (For master and stables)*
            # This needs to be tested in all supported browsers
            # Goto site pages>calendar
            # Try to traverse using a keyboard.
            # Create a few different type of events if you already dont have any
            # make sure the mini calendar on the sidebar displays the popup with event details "on focus" (using keyboard) and "on mouseover" (using mouse)
            # Delete any events corresponding to current date
            # Make sure the mini calendar on the sidebar displays the popup with content saying "No events" "on focus" (using keyboard) and "on mouseover" (using mouse)

            *Test 2 (Just for master)*
            # Make sure for the step 5 and step 7 cases the popup div has "aria-live=off" attribute. Make sure that changes to "aria-live=assertive" when the popup is displayed and changes back to "off" once the popup is invisible.
            # Make sure all links in the mini calendar on the side bar (corrosponding to days) have "aria-controls='.$popupid.'_panel" atrribute
            Hide
            Frédéric Massart added a comment -

            Good to go! Pushing for integration.

            Show
            Frédéric Massart added a comment - Good to go! Pushing for integration.
            Frédéric Massart made changes -
            Status Waiting for peer review [ 10012 ] Waiting for integration review [ 10010 ]
            Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            Hide
            Sam Hemelryk added a comment -

            Hi Ankit,

            I've just been looking at this now.
            These changes look fine thanks although I'm not a fan of the change in context when attaching to the show/hide events it does work.
            Presently I can't get to git.moodle.org so the integration of this issue will have to wait sorry (hopefully fixed when HQ arrives at work).

            In the mean time although not required in this case I do think it would be a good idea to add the dialog role in order to give the aria-live attribute more relevance.
            Not essential as its not required

            BTW http://www.w3.org/WAI/PF/aria-practices/#dialog_tooltip is a good reference for this overlay I think.

            Many thanks
            Sam

            Show
            Sam Hemelryk added a comment - Hi Ankit, I've just been looking at this now. These changes look fine thanks although I'm not a fan of the change in context when attaching to the show/hide events it does work. Presently I can't get to git.moodle.org so the integration of this issue will have to wait sorry (hopefully fixed when HQ arrives at work). In the mean time although not required in this case I do think it would be a good idea to add the dialog role in order to give the aria-live attribute more relevance. Not essential as its not required BTW http://www.w3.org/WAI/PF/aria-practices/#dialog_tooltip is a good reference for this overlay I think. Many thanks Sam
            Hide
            Sam Hemelryk added a comment -

            Integrated now thanks

            Show
            Sam Hemelryk added a comment - Integrated now thanks
            Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Fix Version/s 2.2.6 [ 12372 ]
            Fix Version/s 2.3.3 [ 12373 ]
            Michael de Raadt made changes -
            Tester phalacee
            Jason Fowler made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            Jason Fowler added a comment -

            Worked fine in Firefox, Chrome, and IE8 & 9. Wasn't able to test 2.2 in IE7 though, as I don't have a copy of that

            Show
            Jason Fowler added a comment - Worked fine in Firefox, Chrome, and IE8 & 9. Wasn't able to test 2.2 in IE7 though, as I don't have a copy of that
            Jason Fowler made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            Dan Poltawski added a comment -

            Congratulations, you've done it!

            Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc

            Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

            Show
            Dan Poltawski added a comment - Congratulations, you've done it! Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.
            Dan Poltawski made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 27/Sep/12
            Rajesh Taneja made changes -
            Link This issue caused a regression MDL-36303 [ MDL-36303 ]
            Michael de Raadt made changes -
            Labels api_change triaged triaged
            Eloy Lafuente (stronk7) made changes -
            Fix Version/s STABLE Sprint 24 Alpha [ 12363 ]
            Hide
            Dan Poltawski added a comment -

            Hi Ankit,

            This issue caused a problem where aria-controls value is completely invalid when on the course page block: MDL-42239

            Show
            Dan Poltawski added a comment - Hi Ankit, This issue caused a problem where aria-controls value is completely invalid when on the course page block: MDL-42239
            Dan Poltawski made changes -
            Link This issue has a non-specific relationship to MDL-42239 [ MDL-42239 ]
            Dan Poltawski made changes -
            Link This issue caused a regression MDL-42239 [ MDL-42239 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: