[MDL-31660] Rename and tidy up lib/yui/formslib/formslib.js - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 2.3
Fix Version/s: 2.3
Component/s: Forms Library
Labels:
- patch
- triaged

Testing Instructions:
Hide

Note to tester: This change is related to ~~MDL-31655~~ and ~~MDL-31656~~. To aid your testing, you may want to test all three together. These instructions are additive

These test instructions are from ~~MDL-31315~~. This patch should not change the functionality of that functionality issued in that issue.

Please note, this functionality is not supported in Opera, or Mobile Safari.

You may also like to keep a Javascript Console handy to ensure that no JS errors were reported during the testing.

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 wish to ensure that the feature is still present.
Show
Note to tester: This change is related to MDL-31655 and MDL-31656 . To aid your testing, you may want to test all three together. These instructions are additive These test instructions are from MDL-31315 . This patch should not change the functionality of that functionality issued in that issue. Please note, this functionality is not supported in Opera, or Mobile Safari. You may also like to keep a Javascript Console handy to ensure that no JS errors were reported during the testing. 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 wish to ensure that the feature is still present.
Difficulty:
Easy
Affected Branches:
MOODLE_23_STABLE
Fixed Branches:
MOODLE_23_STABLE
Pull from Repository:
git://git.luns.net.uk/moodle.git
Pull Master Branch:
~~MDL-31660~~-master-3
Pull Master Diff URL:
https://git.luns.net.uk/?p=moodle.git;a=commitdiff;h=94f45dec399dfee06253ac5f16f2e5184f32de88

Description

As discussed in ~~MDL-31315~~ we should ideally rename the formslib change detector to something more specific - for example formchangeprotection

In doing this, I also came across a couple of areas to tidy up:

the unset_changed function isn't required. It was from an early Proof of Concept and somehow made it through review several times
The listeners only need to be called once, not on every change. This should reduce browser footprints slightly

This change should ideally be tested at the same time as ~~MDL-31655~~ and ~~MDL-31656~~ which make modifications to the same functionality

Gliffy Diagrams

Attachments

Issue Links

blocks

MDL-31655 No warning after editing textbox and not saving.

Closed

has a non-specific relationship to

MDL-31315 Warn users before moving away from modified forms

Closed

will help resolve

MDL-31673 Tidy up formchangeprotection javascript

Closed

MDL-31740 Store form change state in a different js variable

Closed

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Andrew Nicols added a comment - 16/Feb/12 5:52 PM

This issue ideally needs to be completed either before ~~MDL-31655~~ and ~~MDL-31656~~, or afterwards to avoid a nasty merge mess.

Show

Andrew Nicols added a comment - 16/Feb/12 5:52 PM This issue ideally needs to be completed either before MDL-31655 and MDL-31656 , or afterwards to avoid a nasty merge mess.

Hide

Permalink

Andrew Nicols added a comment - 16/Feb/12 10:32 PM

I've got this ready in the pipeline and will rebase on the weeklies when they're available.

Show

Andrew Nicols added a comment - 16/Feb/12 10:32 PM I've got this ready in the pipeline and will rebase on the weeklies when they're available.

Hide

Permalink

Andrew Nicols added a comment - 17/Feb/12 3:58 PM - edited

This is just a basic rename as requested by samh in ~~MDL-31315~~
Submitting for peer review

Show

Andrew Nicols added a comment - 17/Feb/12 3:58 PM - edited This is just a basic rename as requested by samh in MDL-31315 Submitting for peer review

Hide

Permalink

Dan Poltawski added a comment - 20/Feb/12 9:51 AM

Looks fine.

To those who are not familiar with the linked issues this formslib.js was introduced in the last integration cycle - so this is changing a brand new 'regression' from the past issue. It won't be impacting historical things.

Show

Dan Poltawski added a comment - 20/Feb/12 9:51 AM Looks fine. To those who are not familiar with the linked issues this formslib.js was introduced in the last integration cycle - so this is changing a brand new 'regression' from the past issue. It won't be impacting historical things.

Hide

Permalink

Andrew Nicols added a comment - 20/Feb/12 7:38 PM

Holding off on putting this up for Integration Review until ~~MDL-31655~~ and ~~MDL-31673~~ have been looked at. These should ideally all be submitted together as they should be tested together and are all related.

Show

Andrew Nicols added a comment - 20/Feb/12 7:38 PM Holding off on putting this up for Integration Review until MDL-31655 and MDL-31673 have been looked at. These should ideally all be submitted together as they should be tested together and are all related.

Hide

Permalink

Andrew Nicols added a comment - 28/Feb/12 11:45 PM

I've updated this patch to take on the comments in ~~MDL-31673~~ and ~~MDL-31740~~ (which I'll close). This patch now:

renames formslib to formchangechecker
moves all functions out of lib/javascript-static.js into the formchangechecker namespace
stores state information under the formchangechecker namespace
converts the .on to .once (as discussed in ~~MDL-31673~~)
removes the unset_changed function which isn't required

Show

Andrew Nicols added a comment - 28/Feb/12 11:45 PM I've updated this patch to take on the comments in MDL-31673 and MDL-31740 (which I'll close). This patch now: renames formslib to formchangechecker moves all functions out of lib/javascript-static.js into the formchangechecker namespace stores state information under the formchangechecker namespace converts the .on to .once (as discussed in MDL-31673 ) removes the unset_changed function which isn't required

Hide

Permalink

Andrew Nicols added a comment - 28/Feb/12 11:47 PM

Hi Dan,

This probably needs to be re-peer reviewed as I've made some changes to the way in which it is instantiated, and I've merged in the changes discussed in ~~MDL-31673~~ and ~~MDL-31740~~

Show

Andrew Nicols added a comment - 28/Feb/12 11:47 PM Hi Dan, This probably needs to be re-peer reviewed as I've made some changes to the way in which it is instantiated, and I've merged in the changes discussed in MDL-31673 and MDL-31740

Hide

Permalink

Dan Poltawski added a comment - 29/Feb/12 8:33 AM - edited

Hi Andrew,

Looks good. Some comments - you have a few variables which are not following the coding guidelines and have underscores:

form_changed
form_submitted

By removing the underscores (and ensuring underscores in function names as separators) it makes it easier to distinguish between functions and variables.

There is also the variable returnValue - I understand e.returnValue is there for what the browser expects but our private variable doesn't have to be named camel case I don't think.

[but these are just minor comments]

Show

Dan Poltawski added a comment - 29/Feb/12 8:33 AM - edited Hi Andrew, Looks good. Some comments - you have a few variables which are not following the coding guidelines and have underscores: form_changed form_submitted By removing the underscores (and ensuring underscores in function names as separators) it makes it easier to distinguish between functions and variables. There is also the variable returnValue - I understand e.returnValue is there for what the browser expects but our private variable doesn't have to be named camel case I don't think. [but these are just minor comments]

Hide

Permalink

Andrew Nicols added a comment - 29/Feb/12 11:01 PM

Updated with comments suggested by Dan

Show

Andrew Nicols added a comment - 29/Feb/12 11:01 PM Updated with comments suggested by Dan

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 01/Mar/12 7:45 PM

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 - 01/Mar/12 7:45 PM 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

Permalink

Andrew Nicols added a comment - 01/Mar/12 8:22 PM

Rebases cleanly onto latest master

Show

Andrew Nicols added a comment - 01/Mar/12 8:22 PM Rebases cleanly onto latest master

Hide

Permalink

Sam Hemelryk added a comment - 05/Mar/12 6:00 AM

Awesome thanks Andrew - this has been integrated now!

Cheers
Sam

Show

Sam Hemelryk added a comment - 05/Mar/12 6:00 AM Awesome thanks Andrew - this has been integrated now! Cheers Sam

Hide

Permalink

Andrew Davis added a comment - 07/Mar/12 10:43 AM

Phew. This all works as described. Passing.

Show

Andrew Davis added a comment - 07/Mar/12 10:43 AM Phew. This all works as described. Passing.

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 10/Mar/12 5:28 AM

Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!).

icao_reverse('arreis olik rebemevon afla letoh ognat');

Closing, ciao

Show

Eloy Lafuente (stronk7) added a comment - 10/Mar/12 5:28 AM Your changes are now upstream and will be included in the next minor released scheduled for March 13th (next Monday!). icao_reverse('arreis olik rebemevon afla letoh ognat'); Closing, ciao

People

Assignee:

Andrew Nicols

Reporter:

Andrew Nicols

Peer reviewer:

Dan Poltawski

Integrator:

Sam Hemelryk

Tester:

Andrew Davis

Participants:

Andrew Davis, Andrew Nicols, Dan Poltawski, Eloy Lafuente (stronk7), Sam Hemelryk

Votes:

0 Vote for this issue

Watchers:

1 Start watching this issue

Dates

Created:

16/Feb/12 5:39 PM

Updated:

10/Mar/12 5:28 AM

Resolved:

10/Mar/12 5:28 AM

Fix Release Date:

25/Jun/12

Details

Without any changes

Now make some changes to various fields

Go back to the course and create an Assignment

Without any changes

Make some changes to the TinyMCE editor

Uploading files

Modify existing files

Admin Pages

Description

Gliffy Diagrams

Attachments

Issue Links

Activity

People

Dates