Details

    • Testing Instructions:
      Hide
      1. enable tag functionality in advanced features
      2. Make sure to have some tag available within the site (tag could be added through blog)
      3. go to site admin > apprearance > manage tags
      4. change the option for tag type to official
      5. change the option for 'with selected tags..' to 'change tag type' and hit ok

      Make sure there's no error display and make sure the text and select input has label.

      Show
      enable tag functionality in advanced features Make sure to have some tag available within the site (tag could be added through blog) go to site admin > apprearance > manage tags change the option for tag type to official change the option for 'with selected tags..' to 'change tag type' and hit ok Make sure there's no error display and make sure the text and select input has label.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34573_accessibility
    • Rank:
      43006

      Description

      There are several select input fields that do not have labels explicitly tied to them. Often this is because there is a visual cue as to what information is being asked for but the visual cue is not explicitly linked to the input element.

      Because this problem is sporadic it might be necessary to break this task out into smaller sub-tasks for each instance of the problem.

      Here is a tutorial for methods of labeling text input elements. http://oit.ncsu.edu/itaccess/forms#select

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          Hmm. I'm not so sure about <label for="coursetag_new_tag">$edittagthisunit</label>

          Show
          Dan Poltawski added a comment - Hmm. I'm not so sure about <label for="coursetag_new_tag">$edittagthisunit</label>
          Hide
          Dan Poltawski added a comment -

          We have multiple labels with the same name?

          Show
          Dan Poltawski added a comment - We have multiple labels with the same name?
          Hide
          Dan Poltawski added a comment -

          I'm reopening this - it needs testing instructions for the coursetags_edit.php change. And upon looking at this change there its not correct. In my browser I see the hidden label, and its not right. The input is also disabeld. It needs looking at anyway.

          Show
          Dan Poltawski added a comment - I'm reopening this - it needs testing instructions for the coursetags_edit.php change. And upon looking at this change there its not correct. In my browser I see the hidden label, and its not right. The input is also disabeld. It needs looking at anyway.
          Hide
          Michael de Raadt added a comment -

          Carrying over to the next sprint.

          Show
          Michael de Raadt added a comment - Carrying over to the next sprint.
          Hide
          Rossiani Wijaya added a comment -

          Hi Dan,

          The input is set to disabled because it is used to display the suggested tag as user typing in on the other input text.

          There's a typo on the hidden label class.

          I updated the patch to create new label for suggested tag, fixed the typo for the hidden label and re-arranging the display for the label and input (there's no visible changes for this).

          Show
          Rossiani Wijaya added a comment - Hi Dan, The input is set to disabled because it is used to display the suggested tag as user typing in on the other input text. There's a typo on the hidden label class. I updated the patch to create new label for suggested tag, fixed the typo for the hidden label and re-arranging the display for the label and input (there's no visible changes for this).
          Hide
          David Monllaó added a comment -

          The changes looks ok, I've also looked for other select and text elements without label

          Show
          David Monllaó added a comment - The changes looks ok, I've also looked for other select and text elements without label
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame some-body/thing/where, don't do it here (use git instead) :-D :-P

          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame some-body/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

          This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

          Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Aparup Banerjee added a comment -

          reopening (as discussed with Rossi), to use html_writer where possible.

          Show
          Aparup Banerjee added a comment - reopening (as discussed with Rossi), to use html_writer where possible.
          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 -

          Update patch for master to use html_writer for manage.php only.

          Show
          Rossiani Wijaya added a comment - Update patch for master to use html_writer for manage.php only.
          Hide
          Frédéric Massart added a comment -

          Hi Rosie,

          Sorry but I still there is still work to be done. But I am pretty sure it should not be done in the scope of this issue.

          The problem I see is that using html_writer all over the place is an undeniable improvement, but half way through of what it should be. For example, html_writer::link() should get a moodle_url() and not a hardcoded url. Also, a form should never be handmade and should use a proper moodle_form().

          What I would suggest (but I am no authority here) would be to create a new issue to transform the tag/manage.php page into a proper clean page using our new standards. I would then remove the commit introducing html_writer but keep it for the new issue.

          Show
          Frédéric Massart added a comment - Hi Rosie, Sorry but I still there is still work to be done. But I am pretty sure it should not be done in the scope of this issue. The problem I see is that using html_writer all over the place is an undeniable improvement, but half way through of what it should be. For example, html_writer::link() should get a moodle_url() and not a hardcoded url. Also, a form should never be handmade and should use a proper moodle_form(). What I would suggest (but I am no authority here) would be to create a new issue to transform the tag/manage.php page into a proper clean page using our new standards. I would then remove the commit introducing html_writer but keep it for the new issue.
          Hide
          Michael de Raadt added a comment -

          I think I liked the simpler, minimal change. Either we make a simple change, consistent with the surrounding code, or we re-write this page with mforms. Perhaps we can do the minimal change now and create an issue for updating the interface at a later stage.

          Show
          Michael de Raadt added a comment - I think I liked the simpler, minimal change. Either we make a simple change, consistent with the surrounding code, or we re-write this page with mforms. Perhaps we can do the minimal change now and create an issue for updating the interface at a later stage.
          Hide
          Rossiani Wijaya added a comment -

          Thanks Fred for reviewing.

          Update patch to use moodle_url() for html_writer::link().

          As for converting to use mform, it is an improvement for the code and it is beyond the scope of this issue. I create MDL-35474 to address the issue.

          Also, I spoke with Apu last week and we agreed to leave the '<form>' tag as it is and convert the '<input> tag to use html_writer.

          Sending for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks Fred for reviewing. Update patch to use moodle_url() for html_writer::link(). As for converting to use mform, it is an improvement for the code and it is beyond the scope of this issue. I create MDL-35474 to address the issue. Also, I spoke with Apu last week and we agreed to leave the '<form>' tag as it is and convert the '<input> tag to use html_writer. Sending for integration review.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Sam Hemelryk added a comment -

          Thanks Rossi, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Rossi, this has been integrated now
          Hide
          Dan Poltawski added a comment -

          I get this error testing MDL-15471, I think it might be related:

          "Invalid key name in optional_param_array() detected: "3", parameter: newname
          line 657 of /lib/moodlelib.php: call to debugging()
          line 33 of /tag/manage.php: call to optional_param_array()
          Invalid key name in optional_param_array() detected: "4", parameter: newname
          line 657 of /lib/moodlelib.php: call to debugging()
          line 33 of /tag/manage.php: call to optional_param_array()"

          Show
          Dan Poltawski added a comment - I get this error testing MDL-15471 , I think it might be related: "Invalid key name in optional_param_array() detected: "3", parameter: newname line 657 of /lib/moodlelib.php: call to debugging() line 33 of /tag/manage.php: call to optional_param_array() Invalid key name in optional_param_array() detected: "4", parameter: newname line 657 of /lib/moodlelib.php: call to debugging() line 33 of /tag/manage.php: call to optional_param_array()"
          Hide
          Dan Poltawski added a comment -

          Failing this test as i've confirmed this issue introduced the regresison.

          1/ Create some tags
          2/ Go to the tags manage page
          3/ Change one of the tag types to official
          4/ Chose 'change tag type' setting and Submit
          Error occurs:

          Invalid key name in optional_param_array() detected: "8", parameter: newname
          line 657 of /lib/moodlelib.php: call to debugging()
          line 33 of /tag/manage.php: call to optional_param_array()

          Show
          Dan Poltawski added a comment - Failing this test as i've confirmed this issue introduced the regresison. 1/ Create some tags 2/ Go to the tags manage page 3/ Change one of the tag types to official 4/ Chose 'change tag type' setting and Submit Error occurs: Invalid key name in optional_param_array() detected: "8", parameter: newname line 657 of /lib/moodlelib.php: call to debugging() line 33 of /tag/manage.php: call to optional_param_array()
          Hide
          Rossiani Wijaya added a comment -

          Hi Dan,

          The issue is caused by additional quotes for the new tag name field.

          I updated the patch to remove those quotes. This is only occur on master.

          Show
          Rossiani Wijaya added a comment - Hi Dan, The issue is caused by additional quotes for the new tag name field. I updated the patch to remove those quotes. This is only occur on master.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Added new commit fixing those tag keys (master only). Re-sending to testing... thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Added new commit fixing those tag keys (master only). Re-sending to testing... thanks!
          Hide
          Andrew Davis added a comment -

          I've tested the steps Dan provided and can no longer reproduce that problem.

          Show
          Andrew Davis added a comment - I've tested the steps Dan provided and can no longer reproduce that problem.
          Hide
          Andrew Davis added a comment -

          All seems fine. Passing.

          Show
          Andrew Davis added a comment - All seems fine. Passing.
          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.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: