Moodle
  1. Moodle
  2. MDL-34352

chooserdialogue shouldn't access form elements by id

    Details

    • Testing Instructions:
      Hide

      It's advisable to have your JS console open for these tests

      • Open a course page
      • Turn editing on
      • Open the Activity Chooser
      • Hit the Cancel button
        • Confirm that the submit button is disabled
        • Confirm that the chooser closes correctly without error
      • Re-open the chooser
        • Confirm that the submit button is disabled
      • Select a module
        • Confirm that the submit button is now enabled
      • Click the submit button
        • Confirm that the correct modedit page opens
      Show
      It's advisable to have your JS console open for these tests Open a course page Turn editing on Open the Activity Chooser Hit the Cancel button Confirm that the submit button is disabled Confirm that the chooser closes correctly without error Re-open the chooser Confirm that the submit button is disabled Select a module Confirm that the submit button is now enabled Click the submit button Confirm that the correct modedit page opens
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34352-master-2
    • Rank:
      42724

      Description

      It's possible (and likely) that there will be more than one chooserdialogue available at a time. At present, we're using IDs to access certain fields:

      • jumplink
      • submitbutton
      • cancelbutton

      But it's a breach of the XHTML spec to use the same ID multiple times.

      We need to use class selectors rather than IDs

        Issue Links

          Activity

          Hide
          Frédéric Massart added a comment -

          Code looks good Andrew. Pushing for integration!

          Show
          Frédéric Massart added a comment - Code looks good Andrew. Pushing for integration!
          Hide
          Sam Hemelryk added a comment - - edited

          Umm sorry, just one thing to check up / clarify.
          It looks like ./course/yui/modchooser/modchooser.js is still referencing #jump.

          Show
          Sam Hemelryk added a comment - - edited Umm sorry, just one thing to check up / clarify. It looks like ./course/yui/modchooser/modchooser.js is still referencing #jump.
          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 -

          Sorry, sending this back Andrew it looks to me like that modchooser.js needs to be updated as well.
          Unfortunately I am neither 100% sure or confident enough to make the changes during integration and I think its best you have a look at it yourself.
          Feel free to correct it and push it back up for integration, or correct me

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Sorry, sending this back Andrew it looks to me like that modchooser.js needs to be updated as well. Unfortunately I am neither 100% sure or confident enough to make the changes during integration and I think its best you have a look at it yourself. Feel free to correct it and push it back up for integration, or correct me 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
          Andrew Nicols added a comment -

          Apologies Sam,

          When I wrote this I was also working on another related commit.
          The assigning of the jumplink in modchooser is actually superflous as it's set in chooserdialogue::display_chooser() so I've removed it - I'm going to be doing additional work on this in another issue to improve things further.

          Show
          Andrew Nicols added a comment - Apologies Sam, When I wrote this I was also working on another related commit. The assigning of the jumplink in modchooser is actually superflous as it's set in chooserdialogue::display_chooser() so I've removed it - I'm going to be doing additional work on this in another issue to improve things further.
          Hide
          Sam Hemelryk added a comment -

          Cool thanks for clearing that up Andrew, looking at this again now

          Show
          Sam Hemelryk added a comment - Cool thanks for clearing that up Andrew, looking at this again now
          Hide
          Sam Hemelryk added a comment -

          Thanks Andrew, things look spot on. This has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks Andrew, things look spot on. This has been integrated now.
          Hide
          Adrian Greeve added a comment -

          I confirmed that the submit button was disabled when it should be and enabled when an activity or resourse is selected.
          I'm always re-directed to the correct modedit page as well.
          No error were discovered during the testing process.
          Test passed

          Show
          Adrian Greeve added a comment - I confirmed that the submit button was disabled when it should be and enabled when an activity or resourse is selected. I'm always re-directed to the correct modedit page as well. No error were discovered during the testing process. Test passed
          Hide
          Aparup Banerjee added a comment -

          yay, it works!

          This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week.

          Thank you all for taking the time to get us here.

          cheers!

          Show
          Aparup Banerjee added a comment - yay, it works! This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week. Thank you all for taking the time to get us here. cheers!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: