Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.2
    • Fix Version/s: 2.4
    • Component/s: Forms Library
    • Labels:
    • Testing Instructions:
      Hide

      These are regressions tests. It is very difficult to test the performance improvements short of breaking out your favourite profiling tool. On the forms I was testing on (Edit course settings, frontpage settings, add new forum), the improvement was in the region of 10ms.

      These test instructions are more-or-less the same as when the checker was introduced.

      Open the Moodle Login Page

      • Make a change to to the username
      • Hit Refresh
      • The page refreshes

      Log in to Moodle and navigate to a course

      • Choose Edit Settings
      Without any changes
      • Refresh the page (the page refreshes)
      • Hit the back button (you go back in time and space)
      • Go back to the Edit Settings Page
      • Try closing the browser window (it closes)
      • Click Save and Display (the form saves)
      Now make some changes to various fields
      • Go back to the Edit Settings Page
      • Make a change to the 'Course full name'
      • Refresh the page (you get a popup) - click to stay
      • Hit the back button (you get a popup) - click to stay
      • Try closing the browser window/tab (you get a popup) - click to stay
      • Click Save and Display (the form saves)

      Repeat the above steps using a select instead of 'Course full name'
      Repeat the above steps using a textarea (not TinyMCE) instead of 'Course full name'

      Go back to the course and create an Assignment
      • Choose Edit Settings
      Without any changes
      • Refresh the page to confirm that it refreshes when there have been no changes
      Make some changes to the TinyMCE editor
      • Refresh the page (you get a popup) - click to stay
      • Hit the back button (you get a popup) - click to stay
      • Try closing the browser window/tab (you get a popup) - click to stay
      • Click Save and Display (the form saves)
      Uploading files
      • As a Student, choose the 'Upload Files' option
      • Refresh the page to confirm that it refreshes when there have been no changes
      • Upload a file using the 'Add' button and the filepicker
      • Refresh the page (you get a popup) - click to stay
      • Hit the back button (you get a popup) - click to stay
      • Try closing the browser window/tab (you get a popup) - click to stay
      • Refresh the page and this time leave to start afresh
      • Upload a file using the drag/drop functionality (Supported browsers only - Chrome and Firefox)
      • Refresh the page (you get a popup) - click to stay
      • Hit the back button (you get a popup) - click to stay
      • Try closing the browser window/tab (you get a popup) - click to stay
      • Click 'Save Changes'
      Modify existing files
      • Choose the 'Edit these files' button
      • Refresh the page and this time leave to start afresh
      • From the drop-down on a file, choose the rename option and enter a new name
      • Upload a file using the drag/drop functionality (Supported browsers only - Chrome and Firefox)
      • Refresh the page (you get a popup) - click to stay
      • Hit the back button (you get a popup) - click to stay
      • Try closing the browser window/tab (you get a popup) - click to stay
      • Refresh the page and this time leave to start afresh
      • From the drop-down menu on a file, choose the Delete option and confirm
      • Refresh the page (you get a popup) - click to stay
      • Hit the back button (you get a popup) - click to stay
      • Try closing the browser window/tab (you get a popup) - click to stay
      • Refresh the page and this time leave to start afresh
      • Create a new directory
      • From the drop-down menu on a file, choose the Delete option and confirm
      • Refresh the page (you get a popup) - click to stay
      • Hit the back button (you get a popup) - click to stay
      • Try closing the browser window/tab (you get a popup) - click to stay
      • Refresh the page and this time leave to start afresh
      • Add an additional files using either option
      • From the drop-down menu on a file, choose the Delete option and confirm
      • Refresh the page (you get a popup) - click to stay
      • Hit the back button (you get a popup) - click to stay
      • Try closing the browser window/tab (you get a popup) - click to stay
      Admin Pages

      These changes are also included in admin/settings.php pages. The same code is used throughout so it's not really necessary to test all functionality again, but you may check if you wish.

      Show
      These are regressions tests. It is very difficult to test the performance improvements short of breaking out your favourite profiling tool. On the forms I was testing on (Edit course settings, frontpage settings, add new forum), the improvement was in the region of 10ms. These test instructions are more-or-less the same as when the checker was introduced. Open the Moodle Login Page Make a change to to the username Hit Refresh The page refreshes Log in to Moodle and navigate to a course Choose Edit Settings Without any changes Refresh the page (the page refreshes) Hit the back button (you go back in time and space) Go back to the Edit Settings Page Try closing the browser window (it closes) Click Save and Display (the form saves) Now make some changes to various fields Go back to the Edit Settings Page Make a change to the 'Course full name' Refresh the page (you get a popup) - click to stay Hit the back button (you get a popup) - click to stay Try closing the browser window/tab (you get a popup) - click to stay Click Save and Display (the form saves) Repeat the above steps using a select instead of 'Course full name' Repeat the above steps using a textarea (not TinyMCE) instead of 'Course full name' Go back to the course and create an Assignment Choose Edit Settings Without any changes Refresh the page to confirm that it refreshes when there have been no changes Make some changes to the TinyMCE editor Refresh the page (you get a popup) - click to stay Hit the back button (you get a popup) - click to stay Try closing the browser window/tab (you get a popup) - click to stay Click Save and Display (the form saves) Uploading files As a Student, choose the 'Upload Files' option Refresh the page to confirm that it refreshes when there have been no changes Upload a file using the 'Add' button and the filepicker Refresh the page (you get a popup) - click to stay Hit the back button (you get a popup) - click to stay Try closing the browser window/tab (you get a popup) - click to stay Refresh the page and this time leave to start afresh Upload a file using the drag/drop functionality (Supported browsers only - Chrome and Firefox) Refresh the page (you get a popup) - click to stay Hit the back button (you get a popup) - click to stay Try closing the browser window/tab (you get a popup) - click to stay Click 'Save Changes' Modify existing files Choose the 'Edit these files' button Refresh the page and this time leave to start afresh From the drop-down on a file, choose the rename option and enter a new name Upload a file using the drag/drop functionality (Supported browsers only - Chrome and Firefox) Refresh the page (you get a popup) - click to stay Hit the back button (you get a popup) - click to stay Try closing the browser window/tab (you get a popup) - click to stay Refresh the page and this time leave to start afresh From the drop-down menu on a file, choose the Delete option and confirm Refresh the page (you get a popup) - click to stay Hit the back button (you get a popup) - click to stay Try closing the browser window/tab (you get a popup) - click to stay Refresh the page and this time leave to start afresh Create a new directory From the drop-down menu on a file, choose the Delete option and confirm Refresh the page (you get a popup) - click to stay Hit the back button (you get a popup) - click to stay Try closing the browser window/tab (you get a popup) - click to stay Refresh the page and this time leave to start afresh Add an additional files using either option From the drop-down menu on a file, choose the Delete option and confirm Refresh the page (you get a popup) - click to stay Hit the back button (you get a popup) - click to stay Try closing the browser window/tab (you get a popup) - click to stay Admin Pages These changes are also included in admin/settings.php pages. The same code is used throughout so it's not really necessary to test all functionality again, but you may check if you wish.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-35673-master
    • Rank:
      44410

      Description

      lib/yui/formchangechecker/formchangechecker.js does code like

      Y.all(formid + ' input').on(...)

      when it would be much better to use Y.delegate(...)

        Activity

        Hide
        Tim Hunt added a comment -

        Assigning to Andrew, since he asked for it.

        Show
        Tim Hunt added a comment - Assigning to Andrew, since he asked for it.
        Hide
        Andrew Nicols added a comment -

        Started work on this.

        Show
        Andrew Nicols added a comment - Started work on this.
        Hide
        Andrew Nicols added a comment -

        This patch is against master only. Given the minimal performance enhancement, I don't feel it's worth backporting to 2.3 but I can provide a patch if required (it should cherry-pick cleanly in any case).

        Show
        Andrew Nicols added a comment - This patch is against master only. Given the minimal performance enhancement, I don't feel it's worth backporting to 2.3 but I can provide a patch if required (it should cherry-pick cleanly in any case).
        Hide
        Tim Hunt added a comment -

        My +1, based on reviewing the code. I have not tested.

        For future reference, you might like to note Y.Array.each, which you could have used instead of your while loop.

        Show
        Tim Hunt added a comment - My +1, based on reviewing the code. I have not tested. For future reference, you might like to note Y.Array.each, which you could have used instead of your while loop.
        Hide
        Andrew Nicols added a comment -

        Thanks Tim - the while loop in this case shifts elements off the array so that they're removed at the same time. This isn't a major thing in this case really though.

        Show
        Andrew Nicols added a comment - Thanks Tim - the while loop in this case shifts elements off the array so that they're removed at the same time. This isn't a major thing in this case really though.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
        Hide
        Rajesh Taneja added a comment -

        Thanks Andrew,

        Works Grt.

        Show
        Rajesh Taneja added a comment - Thanks Andrew, Works Grt.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Closing as fixed, many thanks for your awesome collaboration.

        Show
        Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your awesome collaboration.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: