Moodle
  1. Moodle
  2. MDL-37668

Accessibility: Help popup keyboard focus is unhelpful

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.3
    • Fix Version/s: 2.3.5, 2.4.2
    • Component/s: Accessibility
    • Labels:
      None
    • Testing Instructions:
      Hide

      1. Go to any course webpage.
      2. Using the keyboard, tab to a Help icon. For example, I did this as follows: clicked in the 'Search forums' box, then pressed Tab 3 times.
      3. Press Return to trigger the Help popup.
      EXPECTED: The heading at the top of the popup should have keyboard focus.
      4. Press Tab.
      EXPECTED: The Close button should have keyboard focus.
      5. Press Return again.
      EXPECTED: The popup should close. Focus should be back on the help icon (same as before step 3).

      Show
      1. Go to any course webpage. 2. Using the keyboard, tab to a Help icon. For example, I did this as follows: clicked in the 'Search forums' box, then pressed Tab 3 times. 3. Press Return to trigger the Help popup. EXPECTED: The heading at the top of the popup should have keyboard focus. 4. Press Tab. EXPECTED: The Close button should have keyboard focus. 5. Press Return again. EXPECTED: The popup should close. Focus should be back on the help icon (same as before step 3).
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-37668-m24
    • Pull Master Branch:
      MDL-375668-master
    • Rank:
      47373

      Description

      (Problem reported during routine accessibility testing.)

      When you select a Help popup with the keyboard, it is more difficult than it should be to close the popup. The keyboard focus remains on the link you clicked to open the popup, and when you press Tab, you have to tab through all the other links in the page before you get to the Close button.

      One possibility would be to focus the Close button. However, this might cause problems for screenreader users because the Close button is after the content of the box, so they might find the screenreader does not read the box. I am not certain because the body text is marked with Aria role='alert' so it might work OK anyway... Hmm....

      Another solution is to focus the top of the box, such as the heading (which we can make focusable in JavaScript).

      That way, the keyboard sequence to open and then close the help popup should be Return (popup opens), Tab (focus now on close button), Return. As opposed to Return (popup opens), Tab x n where n = very large number, Return.

      I will investigate how to fix this.

        Issue Links

          Activity

          Hide
          Andrew Nicols added a comment -

          Will be addressed in MDL-35819 for master with a bit of luck...

          Show
          Andrew Nicols added a comment - Will be addressed in MDL-35819 for master with a bit of luck...
          Hide
          Sam Marshall added a comment -

          Thanks Andrew - well spotted!

          I am still going to investigate this for a minimal fix in 2.3 and 2.4 if that's okay (since I got quite a lot of the way through it). Your larger planned improvement looks great though.

          Show
          Sam Marshall added a comment - Thanks Andrew - well spotted! I am still going to investigate this for a minimal fix in 2.3 and 2.4 if that's okay (since I got quite a lot of the way through it). Your larger planned improvement looks great though.
          Hide
          Sam Marshall added a comment -

          Note: In addition to keyboard usage as above, I also tested this briefly on JAWS screen reader (13.0 using Windows XP and IE8). The behaviour is as expected: when you select the help icon, JAWS immediately starts reading the content of the popup.

          Show
          Sam Marshall added a comment - Note: In addition to keyboard usage as above, I also tested this briefly on JAWS screen reader (13.0 using Windows XP and IE8). The behaviour is as expected: when you select the help icon, JAWS immediately starts reading the content of the popup.
          Hide
          Sam Marshall added a comment -

          Also note: don't anyone look at this yet, I'm still tweaking it.

          Show
          Sam Marshall added a comment - Also note: don't anyone look at this yet, I'm still tweaking it.
          Hide
          Sam Marshall added a comment -

          Please could you review this? To explain the two changes:

          1. When closing the box, this used to call the overlay.hide function and not the (already existing) close function. Changed to use existing close function which both hides the overlay and also sets focus back to the help icon.

          2. When content is loaded, the new code finds the <h1> tag (if present), makes it focusable, and focuses it.

          I checked it in current Firefox, IE7, and IE10, seems OK.

          Show
          Sam Marshall added a comment - Please could you review this? To explain the two changes: 1. When closing the box, this used to call the overlay.hide function and not the (already existing) close function. Changed to use existing close function which both hides the overlay and also sets focus back to the help icon. 2. When content is loaded, the new code finds the <h1> tag (if present), makes it focusable, and focuses it. I checked it in current Firefox, IE7, and IE10, seems OK.
          Hide
          Andrew Nicols added a comment -

          I've just tried it using the MacOS voiceover and it seems to work pretty well.

          Since you're working in this neck of the woods though, it's probably worth removing the duplicate link title in the help icon renderer because some screen readers read both of them (the Mac OS one does).

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

          Syntax:
          Neither of these are particularly major but:

          • multiple var definitions (not really a problem to be honest)
          • contentNode var name doesn't fit with coding guidelines

          Feel free to submit for integration when ready.

          Show
          Andrew Nicols added a comment - I've just tried it using the MacOS voiceover and it seems to work pretty well. Since you're working in this neck of the woods though, it's probably worth removing the duplicate link title in the help icon renderer because some screen readers read both of them (the Mac OS one does). [N] Syntax [Y] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Syntax: Neither of these are particularly major but: multiple var definitions (not really a problem to be honest) contentNode var name doesn't fit with coding guidelines Feel free to submit for integration when ready.
          Hide
          Sam Marshall added a comment -

          Thanks for review! I have fixed the two coding style problems indicated (sorry about the variable name, stupid mistake, I didn't know about the other one but I do now).

          I did NOT change the 'duplicated title' situation with the HTML, because I think that is independent from this issue (it involves the HTML rather than the JavaScript) and it's also complex and possibly insoluble. To outline the problem as I understand it (which could be wrong):

          • The same text (roughly) is included in the title of the link, and also in the alt text of the icon in the link. For this reason, VoiceOver reads it twice.
          • To resolve this problem, we could either remove the title or remove the alt text.
          • I understand that most screenreaders by default do not read link titles, so removing the alt text would solve the problem for VoiceOver but might result in no text at all for the link in other screen readers, which is bad.
          • Removing the title would result in losing that title for sighted users, which is bad.

          So basically I'm going to leave that headache for somebody else.

          Submitting the JS change for integration.

          Show
          Sam Marshall added a comment - Thanks for review! I have fixed the two coding style problems indicated (sorry about the variable name, stupid mistake, I didn't know about the other one but I do now). I did NOT change the 'duplicated title' situation with the HTML, because I think that is independent from this issue (it involves the HTML rather than the JavaScript) and it's also complex and possibly insoluble. To outline the problem as I understand it (which could be wrong): The same text (roughly) is included in the title of the link, and also in the alt text of the icon in the link. For this reason, VoiceOver reads it twice. To resolve this problem, we could either remove the title or remove the alt text. I understand that most screenreaders by default do not read link titles, so removing the alt text would solve the problem for VoiceOver but might result in no text at all for the link in other screen readers, which is bad. Removing the title would result in losing that title for sighted users, which is bad. So basically I'm going to leave that headache for somebody else. Submitting the JS change for integration.
          Hide
          Sam Hemelryk added a comment -

          Thanks Sam, this has been integrated now.

          I agree best to tackle the duplicate title bug as a separate issue.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Thanks Sam, this has been integrated now. I agree best to tackle the duplicate title bug as a separate issue. Many thanks Sam
          Hide
          Jason Fowler added a comment -

          all good, thanks for fixing this Sam

          Show
          Jason Fowler added a comment - all good, thanks for fixing this Sam
          Hide
          Eloy Lafuente (stronk7) added a comment -

          A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed.

          (and won't be revisiting it unless some regression is found)

          Thanks and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed. (and won't be revisiting it unless some regression is found) Thanks and ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: