Moodle
  1. Moodle
  2. MDL-38356

Frozen forms cause javascript error: currentform is null

    Details

    • Testing Instructions:
      Hide

      With JS console open, try on as many browsers as possible.
      Remember that Opera does not support this functionality (unless they've changed their minds on that one)

      Test that the form checker still works:

      • Open a form in moodle (e.g. course editing)
        • Confirm no errors were shown in the JS console
      • Make some changes to the form
      • Refresh the page
        • Confirm that an error message was shown

      Test that it doesn't die a horrible death when it's unable to find a proper form:

      • Copy attached file 'formfreezetest.php' into $CFG->dirroot
      • Open the page containing the form
        • Confirm that no errors were shown in the JS console
      • Find the formchangechecker.js file in your Sources tab
      • Add a breakpoint around the new if (!currentform) line
      • Refresh the page to invoke the breakpoint
        • Step into the test to confirm that the return line is hit
      Show
      With JS console open, try on as many browsers as possible. Remember that Opera does not support this functionality (unless they've changed their minds on that one) Test that the form checker still works: Open a form in moodle (e.g. course editing) Confirm no errors were shown in the JS console Make some changes to the form Refresh the page Confirm that an error message was shown Test that it doesn't die a horrible death when it's unable to find a proper form: Copy attached file 'formfreezetest.php' into $CFG->dirroot Open the page containing the form Confirm that no errors were shown in the JS console Find the formchangechecker.js file in your Sources tab Add a breakpoint around the new if (!currentform) line Refresh the page to invoke the breakpoint Step into the test to confirm that the return line is hit
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-38356-m24
    • Pull Master Branch:
    • Rank:
      48243

      Description

      Frozen forms cause javascript error: currentform is null

      • this is due to fact that no form tag is present for frozen forms, but code in /lib/yui/formchangechecker/formchangechecker.js::init() assumes form present.
        Suggested code fix:
        --- a/lib/yui/formchangechecker/formchangechecker.js
        +++ b/lib/yui/formchangechecker/formchangechecker.js
        @@ -23,6 +23,11 @@ YUI.add('moodle-core-formchangechecker',
         
                             // Add change events to the form elements
                             var currentform = Y.one(formid);
        +                    if (!currentform) {
        +                        return;
        +                    }
                             currentform.delegate('change', M.core_formchangechecker.set_form_changed, 'input', this);
                             currentform.delegate('change', M.core_formchangechecker.set_form_changed, 'textarea', this);
                             currentform.delegate('change', M.core_formchangechecker.set_form_changed, 'select', this);
        

        Activity

        Hide
        Andrew Nicols added a comment -

        Hi Brent,

        Can you provide replication instructions. I'm happy with the notion of the fix and can confirm that it doesn't break unfrozen forms, but I wasn't aware you could freeze an entire form. If you could provide some replication and/or testing instructions, that would be ideal.

        Andrew

        Show
        Andrew Nicols added a comment - Hi Brent, Can you provide replication instructions. I'm happy with the notion of the fix and can confirm that it doesn't break unfrozen forms, but I wasn't aware you could freeze an entire form. If you could provide some replication and/or testing instructions, that would be ideal. Andrew
        Hide
        Andrew Nicols added a comment -

        Brent, if you could provide instructions on how to freeze a form then this can go for Peer Review.

        Thanks for the patch,

        Andrew

        Show
        Andrew Nicols added a comment - Brent, if you could provide instructions on how to freeze a form then this can go for Peer Review. Thanks for the patch, Andrew
        Hide
        Andrew Nicols added a comment -

        (and I'll provide stable branches following peer review)

        Show
        Andrew Nicols added a comment - (and I'll provide stable branches following peer review)
        Hide
        Brent Boghosian added a comment -

        Attached basic test file formfreezetest.php that should be copied into dirroot folder and run with browsers console enabled - to view javascript error.

        Show
        Brent Boghosian added a comment - Attached basic test file formfreezetest.php that should be copied into dirroot folder and run with browsers console enabled - to view javascript error.
        Hide
        Andrew Nicols added a comment -

        Actually, I'm going to peer review this since I'm component lead and didn't write the patch, I just committed it.

        Submitting for Integration Review.

        Show
        Andrew Nicols added a comment - Actually, I'm going to peer review this since I'm component lead and didn't write the patch, I just committed it. Submitting for Integration Review.
        Hide
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) 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
        Aparup Banerjee added a comment -

        Thanks for the patch and test file

        integrated into 23, 24 and master for testing.

        Show
        Aparup Banerjee added a comment - Thanks for the patch and test file integrated into 23, 24 and master for testing.
        Hide
        Andrew Davis added a comment - - edited

        Im getting this in 2.3 in Firefox when I open the course settings form. It works fine in Firefox in master. Caches cleared etc.

        Timestamp: 13/03/13 10:30:45
        Error: ReferenceError: currentform is not defined
        Source File: http://localhost/moodle/int/MOODLE_23_STABLE/theme/yui_combo.php?moodle/1363141836/core/formchangechecker/formchangechecker.js
        Line: 20

        Show
        Andrew Davis added a comment - - edited Im getting this in 2.3 in Firefox when I open the course settings form. It works fine in Firefox in master. Caches cleared etc. Timestamp: 13/03/13 10:30:45 Error: ReferenceError: currentform is not defined Source File: http://localhost/moodle/int/MOODLE_23_STABLE/theme/yui_combo.php?moodle/1363141836/core/formchangechecker/formchangechecker.js Line: 20
        Hide
        Andrew Davis added a comment -

        From chat. It appears to be a firefox only problem.

        (10:44:41) Damyon: I don't get an error in latest integration/master (chrome)
        (10:44:57) aparup: not getting one in saari or ff here 
        (10:45:00) aparup: *sfari
        (10:45:12) andrew: im getting it in Firefox
        (10:45:29) Damyon: what setting is supposed to be frozen ?
        (10:45:34) ***aparup says don't mind my globish
        (10:45:59) aparup: did you page finish loading (slow internet?)
        (10:46:19) rwijaya: i'm getting it in FF too
        
        Show
        Andrew Davis added a comment - From chat. It appears to be a firefox only problem. (10:44:41) Damyon: I don't get an error in latest integration/master (chrome) (10:44:57) aparup: not getting one in saari or ff here (10:45:00) aparup: *sfari (10:45:12) andrew: im getting it in Firefox (10:45:29) Damyon: what setting is supposed to be frozen ? (10:45:34) ***aparup says don't mind my globish (10:45:59) aparup: did you page finish loading (slow internet?) (10:46:19) rwijaya: i'm getting it in FF too
        Hide
        Aparup Banerjee added a comment -

        AndrewD,
        does replacing the check with

        if (typeof currentform === 'undefined')"

        help any?

        Show
        Aparup Banerjee added a comment - AndrewD, does replacing the check with if (typeof currentform === 'undefined')" help any?
        Hide
        Andrew Nicols added a comment -

        Hi all,

        I've just taken another look at this - turns out I may a mistake in the backport to stable branches – my many apologies.

        The patch required for 2.3 is completely different, and the patch for 2.4 slightly different. The patch for master is correct.
        I've pushed commits to both the 2.3 and 2.4 branches with commits which address this.

        Andrew

        Show
        Andrew Nicols added a comment - Hi all, I've just taken another look at this - turns out I may a mistake in the backport to stable branches – my many apologies. The patch required for 2.3 is completely different, and the patch for 2.4 slightly different. The patch for master is correct. I've pushed commits to both the 2.3 and 2.4 branches with commits which address this. Andrew
        Hide
        Aparup Banerjee added a comment -

        Thanks, thats been reverted for 23 and 24 and correct patch integrated.

        (i'll open my eyes wider next time.)

        Show
        Aparup Banerjee added a comment - Thanks, thats been reverted for 23 and 24 and correct patch integrated. (i'll open my eyes wider next time.)
        Hide
        Aparup Banerjee added a comment -

        Andrew is in-flight- testing this now.

        Show
        Aparup Banerjee added a comment - Andrew is in-flight- testing this now.
        Hide
        Aparup Banerjee added a comment -

        thanks. this works for me.

        it was quiet a (debugging) step in for 2.3

        Show
        Aparup Banerjee added a comment - thanks. this works for me. it was quiet a (debugging) step in for 2.3
        Hide
        Eloy Lafuente (stronk7) added a comment -

        This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

        Thanks!

        PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

        Show
        Eloy Lafuente (stronk7) added a comment - This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks! PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: