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

      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

        Gliffy Diagrams

          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: