Moodle
  1. Moodle
  2. MDL-27843 META: Accessibility compliance for 2.x
  3. MDL-33575

Unlabelled checkbox element when the individual users are listed in choice module.

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2, 2.3
    • Fix Version/s: 2.1.8, 2.2.5, 2.3.2
    • Component/s: Choice
    • Labels:
    • Testing Instructions:
      Hide

      As admin/teacher:

      1. create choice activity
      2. set 'privacy of results' to 'publish full results, showing names and their choices"

      As student:

      1. Attempt the choice activity.

      As admin/teacher:

      1. view student responses'
      2. view source and make sure there's label for the checkbox element
      Show
      As admin/teacher: create choice activity set 'privacy of results' to 'publish full results, showing names and their choices" As student: Attempt the choice activity. As admin/teacher: view student responses' view source and make sure there's label for the checkbox element
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      41504

      Description

      In MDL-30816, Greg suggested to do the following imporovement:

      <div class="user"><div class="attemptaction">
        <input type="checkbox" name="attemptid[]" value="3" id="user">
      </div>
      
      <div class="image">
        <a href="http://local/moodle/user/view.php?id=3&course=2"><img width="35" height="35" class="userpicture defaultuserpic" title="Picture of donald duck" alt="Picture of donald duck" src="http://local/moodle/theme/image.php?theme=magazine&image=u%2Ff2&rev=935"></a>
      </div>
      <div class="fullname">
        <label for="user"><a class="username" href="http://local/moodle/user/view.php?id=3&course=2">donald duck</a></label></div><div class="clearfloat"></div>
      </div>
      

        Issue Links

          Activity

          Hide
          Ankit Agarwal added a comment -

          Hi,
          Looks good
          +1 for integration

          Show
          Ankit Agarwal added a comment - Hi, Looks good +1 for integration
          Hide
          Rossiani Wijaya added a comment -

          Thanks Ankit for reviewing.

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks Ankit for reviewing. Submitting for integration review.
          Hide
          Sam Hemelryk added a comment -

          Hi Rossie,
          These branches contain a large set of changes for MDL-30816 as well.
          Does this issue need MDL-30816 integrated first, or is this a mistake in the branches?

          Halting integration review until that is clarified.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Rossie, These branches contain a large set of changes for MDL-30816 as well. Does this issue need MDL-30816 integrated first, or is this a mistake in the branches? Halting integration review until that is clarified. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Hi Rosie, can you please get back to me about the branches and additional commits.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Rosie, can you please get back to me about the branches and additional commits. Cheers Sam
          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
          Rossiani Wijaya added a comment -

          Hi Sam,

          Sorry, I didn't notice your comment earlier.

          This needs MDL-30816 to be integrated first.

          Thanks.

          Show
          Rossiani Wijaya added a comment - Hi Sam, Sorry, I didn't notice your comment earlier. This needs MDL-30816 to be integrated first. Thanks.
          Hide
          Sam Hemelryk added a comment -

          Ok great thanks Rosie, have linked them now and will check out MDL-30816. May have to wait until next release now however.

          Show
          Sam Hemelryk added a comment - Ok great thanks Rosie, have linked them now and will check out MDL-30816 . May have to wait until next release now however.
          Hide
          Rossiani Wijaya added a comment -

          Re-submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Re-submitting for integration review.
          Hide
          Sam Hemelryk added a comment -

          Reopening as the blocking issue MDL-30816 has been reopened.

          Show
          Sam Hemelryk added a comment - Reopening as the blocking issue MDL-30816 has been reopened.
          Hide
          Rossiani Wijaya added a comment -

          Updating patch for MDL-30816 and resubmitting this for integration review.

          Show
          Rossiani Wijaya added a comment - Updating patch for MDL-30816 and resubmitting this for integration review.
          Hide
          Sam Hemelryk added a comment -

          This issue has been integrated this week thanks Rosie.
          Before it was integrated I did make one change, the id you used if far to generic and is used elsewhere so likely to cause XHTML issues.
          I've renamed the id from 'user-'.$user->id to 'attempt-user'.$user->id.

          Also just noting I will open a new issue now to fix the XHTML probs in lesson as there were a few when viewing the results screen.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - This issue has been integrated this week thanks Rosie. Before it was integrated I did make one change, the id you used if far to generic and is used elsewhere so likely to cause XHTML issues. I've renamed the id from 'user-'.$user->id to 'attempt-user'.$user->id . Also just noting I will open a new issue now to fix the XHTML probs in lesson as there were a few when viewing the results screen. Cheers Sam
          Hide
          Rajesh Taneja added a comment -

          Sorry Rossie,

          I am failing it for now as I am not sure if passing a link in label is accessible.

          FYI:
          According to w3c "When a LABEL element receives focus, it passes the focus on to its associated control.".
          So how does this link get interpreted by accessibility tools?

          <label for="attempt-user3"><a href="http://rajesh.moodle.local/im/user/view.php?id=3&amp;course=10" class="username">Student Number 1</a></label>
          

          It might be nice to add label to input element then to a link.

          If you feel otherwise, then please feel to change status to pass.

          Show
          Rajesh Taneja added a comment - Sorry Rossie, I am failing it for now as I am not sure if passing a link in label is accessible. FYI: According to w3c "When a LABEL element receives focus, it passes the focus on to its associated control.". So how does this link get interpreted by accessibility tools? <label for="attempt-user3"><a href="http://rajesh.moodle.local/im/user/view.php?id=3&amp;course=10" class="username">Student Number 1</a></label> It might be nice to add label to input element then to a link. If you feel otherwise, then please feel to change status to pass.
          Hide
          Rossiani Wijaya added a comment -

          Thanks Raj.

          I fixed it as suggested by Greg, as he has much more experiences in this field.

          Adding Greg as watcher and comment for this issue.

          Show
          Rossiani Wijaya added a comment - Thanks Raj. I fixed it as suggested by Greg, as he has much more experiences in this field. Adding Greg as watcher and comment for this issue.
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          There's a good chance we are going to need to make a decision about this soon.
          As its been integrated either it is going to stay or its going to be reverted.

          It is very good thinking Raj, this is potentially fixing one accessibility issue while introducing another.
          When looking at this myself I had imagined/assumed that screen readers would read out the label offering it as a link and then shifting onto the input.
          Rosie I think it is best you test to see what happens. You could do it by creating a test page with just an label containing a link and an input and seeing what your screen reader does.
          If it reads the link out, great, I think this can stay.
          If not or focus gets messed up then we need to re-consider this fix and whether it gets reverted, or whether we leave it in there and open another issue to find a better solution.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, There's a good chance we are going to need to make a decision about this soon. As its been integrated either it is going to stay or its going to be reverted. It is very good thinking Raj, this is potentially fixing one accessibility issue while introducing another. When looking at this myself I had imagined/assumed that screen readers would read out the label offering it as a link and then shifting onto the input. Rosie I think it is best you test to see what happens. You could do it by creating a test page with just an label containing a link and an input and seeing what your screen reader does. If it reads the link out, great, I think this can stay. If not or focus gets messed up then we need to re-consider this fix and whether it gets reverted, or whether we leave it in there and open another issue to find a better solution. Cheers Sam
          Hide
          Rajesh Taneja added a comment -

          Thanks Sam,

          I was reading a bit more about this and I think this needs re-thinking. According to http://www.456bereastreet.com/archive/200711/use_the_label_element_to_make_your_html_forms_accessible/
          When checkboxes and radio buttons have properly associated labels, the label text will also be clickable, thus making the target area much larger and easier to hit. This obviously has usability benefits for all users. So if label text is clickable, then adding a link within clickable text might create confusion.

          Sorry, I am no an expert here, but seems we are making it complicated. Not sure what is wrong in adding label at https://github.com/rwijaya/moodle/commit/27b5983be4c94edc5d0ff367f0f2a96adab8045a#L0R217

          Show
          Rajesh Taneja added a comment - Thanks Sam, I was reading a bit more about this and I think this needs re-thinking. According to http://www.456bereastreet.com/archive/200711/use_the_label_element_to_make_your_html_forms_accessible/ When checkboxes and radio buttons have properly associated labels, the label text will also be clickable, thus making the target area much larger and easier to hit. This obviously has usability benefits for all users. So if label text is clickable, then adding a link within clickable text might create confusion. Sorry, I am no an expert here, but seems we are making it complicated. Not sure what is wrong in adding label at https://github.com/rwijaya/moodle/commit/27b5983be4c94edc5d0ff367f0f2a96adab8045a#L0R217
          Hide
          Sam Hemelryk added a comment -

          Ok cool, thanks for having a look/think about it Rosie.

          If you're OK with it I will revert this changes and we can have a better look in coming weeks.
          Sound OK?

          Show
          Sam Hemelryk added a comment - Ok cool, thanks for having a look/think about it Rosie. If you're OK with it I will revert this changes and we can have a better look in coming weeks. Sound OK?
          Hide
          Rossiani Wijaya added a comment -

          It is fine with me Sam.

          Show
          Rossiani Wijaya added a comment - It is fine with me Sam.
          Hide
          Rossiani Wijaya added a comment -

          Update patch

          Show
          Rossiani Wijaya added a comment - Update patch
          Hide
          Rossiani Wijaya added a comment -

          Hi Sam,

          I updated the patch by setting the label right above the select input.

          Show
          Rossiani Wijaya added a comment - Hi Sam, I updated the patch by setting the label right above the select input.
          Hide
          Sam Hemelryk added a comment -

          Has been reverted now, thanks Rosie.

          Show
          Sam Hemelryk added a comment - Has been reverted now, thanks Rosie.
          Hide
          Rajesh Taneja added a comment -

          Code looks good Rossie,

          I tested this as well.
          Not sure if you would like to consider Sam's comment http://tracker.moodle.org/browse/MDL-33575?focusedCommentId=167439&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-167439

          Show
          Rajesh Taneja added a comment - Code looks good Rossie, I tested this as well. Not sure if you would like to consider Sam's comment http://tracker.moodle.org/browse/MDL-33575?focusedCommentId=167439&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-167439
          Hide
          Rossiani Wijaya added a comment -

          Yes Raj, I wanted to test that with screen reader but my screen reader is not being cooperative right now.

          I'm trying to reconfigure my screen reader and report my finding later.

          Show
          Rossiani Wijaya added a comment - Yes Raj, I wanted to test that with screen reader but my screen reader is not being cooperative right now. I'm trying to reconfigure my screen reader and report my finding later.
          Hide
          Rajesh Taneja added a comment -

          Sorry Rossie,

          I was mentioning about id
          Sam's comment:
          I've renamed the id from 'user-'.$user->id to 'attempt-user'.$user->id.

          Show
          Rajesh Taneja added a comment - Sorry Rossie, I was mentioning about id Sam's comment: I've renamed the id from 'user-'.$user->id to 'attempt-user'.$user->id.
          Hide
          Rossiani Wijaya added a comment -

          Thanks for the reminder Raj.

          Updated the patch and sending for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks for the reminder Raj. Updated the patch and sending for integration review.
          Hide
          Sam Hemelryk added a comment -

          Hi Rosie,
          Can you please rebase these changes, I got conflicts on 21, and 22 branches (did try 23, and master).

          Thanks
          Sam

          Show
          Sam Hemelryk added a comment - Hi Rosie, Can you please rebase these changes, I got conflicts on 21, and 22 branches (did try 23, and master). Thanks Sam
          Hide
          Sam Hemelryk added a comment -

          ping

          Show
          Sam Hemelryk added a comment - ping
          Hide
          Rossiani Wijaya added a comment -

          Sure Sam,

          Fixing it right now.

          Show
          Rossiani Wijaya added a comment - Sure Sam, Fixing it right now.
          Hide
          Rossiani Wijaya added a comment -

          Re-basing patch.

          Thanks Sam.

          Show
          Rossiani Wijaya added a comment - Re-basing patch. Thanks Sam.
          Hide
          Sam Hemelryk added a comment -

          Thanks Rosie, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Rosie, this has been integrated now
          Hide
          Rajesh Taneja added a comment -

          Thanks Rossie,

          Checkbox is labelled correctly.

          Show
          Rajesh Taneja added a comment - Thanks Rossie, Checkbox is labelled correctly.
          Hide
          Dan Poltawski added a comment -

          *Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

          Congratulations

          {tracker.user.name}

          !

          You've made into Moodle

          {tracker.fixversion-1}

          +

          I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

          cheers!

          {tracker.friendlyintegrator}
          Show
          Dan Poltawski added a comment - * Notice *: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26 Congratulations {tracker.user.name} ! You've made into Moodle {tracker.fixversion-1} + I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world. cheers! {tracker.friendlyintegrator}

            People

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

              Dates

              • Created:
                Updated:
                Resolved: