Details

    • Testing Instructions:
      Hide
      1. Flush your cache
      2. Create a new quiz
      3. Make sure the divs containing the Save and Cancel buttons do not contain an empty label, and that the buttons look nice in the page
      4. Navigate to 'Settings > Site administration > Users > Add a user'
      5. Make sure the divs containing the 'Update profile' button does not contain an empty label, and that the button look nice in the page
      6. Navigate to 'Settings > Course administration > Edit settings'
      7. Make sure the divs containing the Cancel and Save button do not contain an empty label, and that the buttons look nice in the page
      8. Repeat with a few other themes.
      Show
      Flush your cache Create a new quiz Make sure the divs containing the Save and Cancel buttons do not contain an empty label, and that the buttons look nice in the page Navigate to 'Settings > Site administration > Users > Add a user' Make sure the divs containing the 'Update profile' button does not contain an empty label, and that the button look nice in the page Navigate to 'Settings > Course administration > Edit settings' Make sure the divs containing the Cancel and Save button do not contain an empty label, and that the buttons look nice in the page Repeat with a few other themes.
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30844-master
    • Rank:
      33844

      Description

      Frequently near the bottoms of pages with input forms there is an orphaned form label with no content and not attached to an input element. This is probably from a loop iterating one time too many when creating the page. Most Web page accessibility tools will easily identify the pages where this problem exists. Depending on if the problem is in a common library or if it exists in lots of different places it might be necessary to break this into sub-tasks.

        Issue Links

          Activity

          Hide
          Greg Kraus added a comment -

          I'll provide some example pages after I enter the rest of the bugs from the report.

          Show
          Greg Kraus added a comment - I'll provide some example pages after I enter the rest of the bugs from the report.
          Hide
          Greg Kraus added a comment -

          None of the assistive technologies I tested actually had problems with this, but it would be nice to clean up the code here.

          Show
          Greg Kraus added a comment - None of the assistive technologies I tested actually had problems with this, but it would be nice to clean up the code here.
          Hide
          Michael de Raadt added a comment -

          Hi, Greg.

          Yes, it would be good to see an example of this.

          Show
          Michael de Raadt added a comment - Hi, Greg. Yes, it would be good to see an example of this.
          Hide
          Greg Kraus added a comment -

          When adding a new file resource, near the bottom of the page you will find the following code.

          <div class="fitem"><div class="fitemtitle"><div class="fgrouplabel"><label> </label></div></div><fieldset class="felement fgroup"><input type="submit" id="id_submitbutton2" value="Save and return to course" name="submitbutton2"> <input type="submit" id="id_submitbutton" value="Save and display" name="submitbutton"> <input type="submit" id="id_cancel" onclick="skipClientValidation = true; return true;" value="Cancel" name="cancel"></fieldset></div>

          Note the "<label> </label>" about a quarter of the way into the line of code. This same pattern seems to exist at the bottom of all of the "add a resource/activity" pages.

          Show
          Greg Kraus added a comment - When adding a new file resource, near the bottom of the page you will find the following code. <div class="fitem"><div class="fitemtitle"><div class="fgrouplabel"><label> </label></div></div><fieldset class="felement fgroup"><input type="submit" id="id_submitbutton2" value="Save and return to course" name="submitbutton2"> <input type="submit" id="id_submitbutton" value="Save and display" name="submitbutton"> <input type="submit" id="id_cancel" onclick="skipClientValidation = true; return true;" value="Cancel" name="cancel"></fieldset></div> Note the "<label> </label>" about a quarter of the way into the line of code. This same pattern seems to exist at the bottom of all of the "add a resource/activity" pages.
          Hide
          Frédéric Massart added a comment -

          Note to reviewer: I noticed that the 'template' used within the form helper did not allow me to remove the label. I first tried to set a label and hide it with CSS, but that did not work really smoothly and did not make much sense. I then decided to add a new template for the action buttons, which would be used when a group only contains submit buttons. I made the assumption that we would not need a label when a group only contains submit elements. Please tell me what you think about this. I will create the 2.1 and 2.2 branches once the code is validated. Thanks!

          Show
          Frédéric Massart added a comment - Note to reviewer: I noticed that the 'template' used within the form helper did not allow me to remove the label. I first tried to set a label and hide it with CSS, but that did not work really smoothly and did not make much sense. I then decided to add a new template for the action buttons, which would be used when a group only contains submit buttons. I made the assumption that we would not need a label when a group only contains submit elements. Please tell me what you think about this. I will create the 2.1 and 2.2 branches once the code is validated. Thanks!
          Hide
          Dan Poltawski added a comment -

          Adding Tim as watcher.

          Show
          Dan Poltawski added a comment - Adding Tim as watcher.
          Hide
          Rajesh Taneja added a comment -

          Thanks Fred,

          Code looks good, but I am not sure how it's going to affect themes/JS (if it's used in some way). I think Tim is the right person to review this.

          Show
          Rajesh Taneja added a comment - Thanks Fred, Code looks good, but I am not sure how it's going to affect themes/JS (if it's used in some way). I think Tim is the right person to review this.
          Hide
          Tim Hunt added a comment -

          The theme point is a good one. How do themes achieve their layout of forms? If the plan is float-left the label, fixed width, with the actual form field float-left beside it, then this change would alter the layout.

          Except ...

          What I say about how it is supposed to work is true. However, when the label is empty, it gets a hight of 0px, so actually it does not push the buttons to the right, at least not in the theme I am looking at. So, your change of code will not actually change the layout of the forms, although it will change how I assume they are supposed to be laid out.

          I will let you decide what to do about this.

          I will just say, well done for spotting my evil hack in comment.php, and fixing that too.

          Show
          Tim Hunt added a comment - The theme point is a good one. How do themes achieve their layout of forms? If the plan is float-left the label, fixed width, with the actual form field float-left beside it, then this change would alter the layout. Except ... What I say about how it is supposed to work is true. However, when the label is empty, it gets a hight of 0px, so actually it does not push the buttons to the right, at least not in the theme I am looking at. So, your change of code will not actually change the layout of the forms, although it will change how I assume they are supposed to be laid out. I will let you decide what to do about this. I will just say, well done for spotting my evil hack in comment.php, and fixing that too.
          Hide
          Frédéric Massart added a comment -

          Raj, what do you think?

          Show
          Frédéric Massart added a comment - Raj, what do you think?
          Hide
          Rajesh Taneja added a comment -

          Fred,
          I think before you push it for integration, please make sure button alignment is not disturbed because of this change. Try check this on different themes and feel free to push it for integration if it looks fine.

          Show
          Rajesh Taneja added a comment - Fred, I think before you push it for integration, please make sure button alignment is not disturbed because of this change. Try check this on different themes and feel free to push it for integration if it looks fine.
          Hide
          Frédéric Massart added a comment -

          I have tested over a bunch of different themes and everything looks fine. I have worked on this issue a few weeks ago, and now remember that the label does not push the form elements to the right, there is a margin set on the fields containers to create the alignment. So removing this empty label should not change anything, except eventually for themes which work totally differently.

          I rebased this, and am pushing it for integration. No branching for 2.x as it contains HTML changes.

          Show
          Frédéric Massart added a comment - I have tested over a bunch of different themes and everything looks fine. I have worked on this issue a few weeks ago, and now remember that the label does not push the form elements to the right, there is a margin set on the fields containers to create the alignment. So removing this empty label should not change anything, except eventually for themes which work totally differently. I rebased this, and am pushing it for integration. No branching for 2.x as it contains HTML changes.
          Hide
          Sam Hemelryk added a comment -

          I've just added the integration_held label and removed this from the current integration run sorry.
          It appears this is a master only change and as we are keeping 23 and master in sync has been delayed until after the minor release this Friday.

          My apologies for any inconvenience caused.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - I've just added the integration_held label and removed this from the current integration run sorry. It appears this is a master only change and as we are keeping 23 and master in sync has been delayed until after the minor release this Friday. My apologies for any inconvenience caused. Cheers Sam
          Hide
          Dan Poltawski added a comment -

          Integrated, thanks

          Show
          Dan Poltawski added a comment - Integrated, thanks
          Hide
          Adrian Greeve added a comment -

          I tested this in master first to get an understanding of the issue, then I tested the integration version. Orphaned form labels are no longer present and the buttons do not look out of place.
          Test passed

          Show
          Adrian Greeve added a comment - I tested this in master first to get an understanding of the issue, then I tested the integration version. Orphaned form labels are no longer present and the buttons do not look out of place. Test passed
          Hide
          Dan Poltawski added a comment -

          Congratulations!

          You've made it into the weekly release!

          Thanks for your contribution - here are some random drummers to keep you inspired for the next week!
          http://www.youtube.com/watch?v=_QhpHUmVCmY

          Show
          Dan Poltawski added a comment - Congratulations! You've made it into the weekly release! Thanks for your contribution - here are some random drummers to keep you inspired for the next week! http://www.youtube.com/watch?v=_QhpHUmVCmY
          Hide
          Tim Hunt added a comment -

          This bug dealt with bogus labels on submits. It seems that the same problem exists for 'static' form elements (e.g. MDL-35968). Does anyone know if any work has been done on that? Thanks.

          Show
          Tim Hunt added a comment - This bug dealt with bogus labels on submits. It seems that the same problem exists for 'static' form elements (e.g. MDL-35968 ). Does anyone know if any work has been done on that? Thanks.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: