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

      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

        Gliffy Diagrams

          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: