Moodle

remove incompatibility between Moodle and Mootools JS framework

Details

  • Type: Improvement Improvement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.8.2
  • Fix Version/s: None
  • Component/s: Forms Library
  • Labels:
    None
  • Affected Branches:
    MOODLE_18_STABLE

Description

To remove errors in the formslib JavaScript I added 3 lines to javascript-static.js. I can't see no problems with those changes. Jamie, might you please check the changes. If hey are ok I will apply them to Moodle 1.8 and 1.9.

Thanks a lot.

Index: javascript-static.js
===================================================================
RCS file: /cvsroot/moodle/moodle/lib/javascript-static.js,v
retrieving revision 1.29.2.3
diff -r1.29.2.3 javascript-static.js
91a92,93
> // change for compatibility with mootools
> if (!dependons.lenght) continue;
120a123,124
> // change for compatibility with mootools
> if(!el.lenght) continue;
130a135,136
> // change for compatibility with mootools
> if (!dependons.lenght) continue;

Attachments

  1. File
    javascript-static_js_03.diff
    13/Jul/07 6:03 AM
    0.7 kB
    Urs Hunkler
  2. File
    javascript-static_js_1.8.2.diff
    11/Jul/07 3:16 AM
    0.5 kB
    Urs Hunkler
  3. File
    javascript-static_js.diff
    11/Jul/07 3:28 PM
    0.6 kB
    Urs Hunkler
  4. File
    javascript-static.js_04.diff
    14/Jul/07 2:25 AM
    1 kB
    Urs Hunkler

Issue Links

This issue will be resolved by:
MDL-10382 Allow removing of elements with disabledIf conditions Minor Closed
This issue has been marked as being related by:
MDL-18569 Formslib disabledif does not work at all in Safari Minor Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jamie Pratt added a comment -

there is a spelling mistake in your code. Change lenght -> length

Show
Jamie Pratt added a comment - there is a spelling mistake in your code. Change lenght -> length
Hide
Permalink
Jamie Pratt added a comment -

Hi Urs,

el is not an array so if(!el.length) continue; will always continue.

Can you give me a link to a page describing why there is this problem with mootools and explaining the solution.

Jamie

Show
Jamie Pratt added a comment - Hi Urs, el is not an array so if(!el.length) continue; will always continue. Can you give me a link to a page describing why there is this problem with mootools and explaining the solution. Jamie
Hide
Permalink
Jamie Pratt added a comment -

oh, sorry, the string object also has a length property. So if(!el.length) should be OK.

Show
Jamie Pratt added a comment - oh, sorry, the string object also has a length property. So if(!el.length) should be OK.
Hide
Permalink
Jamie Pratt added a comment -

Would still like to see a page describing the problem that mootools introduces and the solution.

Show
Jamie Pratt added a comment - Would still like to see a page describing the problem that mootools introduces and the solution.
Hide
Permalink
Urs Hunkler added a comment -

Shame on me . That Typo is a bug and prevented the following lines to be executed. The error couldn't come up this way and the code became useless.

I attach a new diff. I will upload the theme and MooTools to a server where I can give you access. Now I am out till the late afternoon - this evening I'll set all up.

Index: javascript-static.js
===================================================================
RCS file: /cvsroot/moodle/moodle/lib/javascript-static.js,v
retrieving revision 1.29.2.3
diff -r1.29.2.3 javascript-static.js
91a92,93
> // change for compatibility with MooTools
> if ((master === undefined) || (master.value === undefined)) continue;
120a123,124
> // change for compatibility with MooTools
> if((formelement === undefined) || (formelement.disabled === undefined)) continue;
130a135,136
> // change for compatibility with MooTools
> if ((master === undefined) || (master.onclick === undefined)) continue;

Show
Urs Hunkler added a comment - Shame on me . That Typo is a bug and prevented the following lines to be executed. The error couldn't come up this way and the code became useless. I attach a new diff. I will upload the theme and MooTools to a server where I can give you access. Now I am out till the late afternoon - this evening I'll set all up. Index: javascript-static.js =================================================================== RCS file: /cvsroot/moodle/moodle/lib/javascript-static.js,v retrieving revision 1.29.2.3 diff -r1.29.2.3 javascript-static.js 91a92,93 > // change for compatibility with MooTools > if ((master === undefined) || (master.value === undefined)) continue; 120a123,124 > // change for compatibility with MooTools > if((formelement === undefined) || (formelement.disabled === undefined)) continue; 130a135,136 > // change for compatibility with MooTools > if ((master === undefined) || (master.onclick === undefined)) continue;
Hide
Permalink
Urs Hunkler added a comment -

I attach a third patch which works now in FF Opera and Safari. In IE the disable state is not working :: MDL-10440.

Index: javascript-static.js
===================================================================
RCS file: /cvsroot/moodle/moodle/lib/javascript-static.js,v
retrieving revision 1.29.2.3
diff -r1.29.2.3 javascript-static.js
91a92,93
> // change for compatibility with MooTools
> if (master === undefined) break;
108a111,112
> // change for compatibility with MooTools
> if (typeof eltolock != "string") break;
120a125,126
> // change for compatibility with MooTools
> if((formelement === undefined) || (formelement.disabled === undefined)) break;
130a137,138
> // change for compatibility with MooTools
> if (master === undefined) continue;

Show
Urs Hunkler added a comment - I attach a third patch which works now in FF Opera and Safari. In IE the disable state is not working :: MDL-10440. Index: javascript-static.js =================================================================== RCS file: /cvsroot/moodle/moodle/lib/javascript-static.js,v retrieving revision 1.29.2.3 diff -r1.29.2.3 javascript-static.js 91a92,93 > // change for compatibility with MooTools > if (master === undefined) break; 108a111,112 > // change for compatibility with MooTools > if (typeof eltolock != "string") break; 120a125,126 > // change for compatibility with MooTools > if((formelement === undefined) || (formelement.disabled === undefined)) break; 130a137,138 > // change for compatibility with MooTools > if (master === undefined) continue;
Hide
Permalink
Urs Hunkler added a comment -

Version 4.

1) Enhanced Petr's fix for MDL-10382. I wrote an one liner and changed == to === because O'Reilly's "JavaScript The Definite Guide" writes: "When testing for a value to see whether it is undefined, use the === operator, because the == operator treats the undefined value as equal to null".

2) Added "!dependons.propertyIsEnumerable(dependon)" to check for not enumerable properties which may have been added by MooTools.

Checked on Mac, Linux and Windows in Safari, FF 2 and 1.5, IE 6+7 and works with and without the MooTool framework.

Can you Jamie and Petr please check the changes and confirm that they are ok now? The actual patched version is also running on the test site.

Index: javascript-static.js
===================================================================
RCS file: /cvsroot/moodle/moodle/lib/javascript-static.js,v
retrieving revision 1.29.2.4
diff -r1.29.2.4 javascript-static.js
90a91,92
> // change for MooTools compatibility
> if (!dependons.propertyIsEnumerable(dependon)) continue;
92,94c94
< if (master == undefined) { < continue; < }
—
> if (master === undefined) continue;
110a111,112
> // change for MooTools compatibility
> if (!window.webkit && (!dependons[dependon][condition][value].propertyIsEnumerable(ei))) continue;
122a125,126
> // change for MooTools compatibility
> if (!tolock.propertyIsEnumerable(el)) continue;
124,126c128
< if (formelement == undefined) {< continue;< } }
—
> if((formelement === undefined) || (formelement.disabled === undefined)) continue;
135a138
> if (!dependons.propertyIsEnumerable(dependon)) continue;
137,139c140
< if (master == undefined) { < continue; < }
—
> if (master === undefined) continue;

Show
Urs Hunkler added a comment - Version 4. 1) Enhanced Petr's fix for MDL-10382. I wrote an one liner and changed == to === because O'Reilly's "JavaScript The Definite Guide" writes: "When testing for a value to see whether it is undefined, use the === operator, because the == operator treats the undefined value as equal to null". 2) Added "!dependons.propertyIsEnumerable(dependon)" to check for not enumerable properties which may have been added by MooTools. Checked on Mac, Linux and Windows in Safari, FF 2 and 1.5, IE 6+7 and works with and without the MooTool framework. Can you Jamie and Petr please check the changes and confirm that they are ok now? The actual patched version is also running on the test site. Index: javascript-static.js =================================================================== RCS file: /cvsroot/moodle/moodle/lib/javascript-static.js,v retrieving revision 1.29.2.4 diff -r1.29.2.4 javascript-static.js 90a91,92 > // change for MooTools compatibility > if (!dependons.propertyIsEnumerable(dependon)) continue; 92,94c94 < if (master == undefined) { < continue; < } — > if (master === undefined) continue; 110a111,112 > // change for MooTools compatibility > if (!window.webkit && (!dependons[dependon][condition][value].propertyIsEnumerable(ei))) continue; 122a125,126 > // change for MooTools compatibility > if (!tolock.propertyIsEnumerable(el)) continue; 124,126c128 < if (formelement == undefined) {< continue;< } } — > if((formelement === undefined) || (formelement.disabled === undefined)) continue; 135a138 > if (!dependons.propertyIsEnumerable(dependon)) continue; 137,139c140 < if (master == undefined) { < continue; < } — > if (master === undefined) continue;
Hide
Permalink
Tim Hunt added a comment -

What is Mootools? And why is this bug still open if these changes were already made to the code?

http://cvs.moodle.org/moodle/lib/javascript-static.js?r1=1.38&r2=1.39

Show
Tim Hunt added a comment - What is Mootools? And why is this bug still open if these changes were already made to the code? http://cvs.moodle.org/moodle/lib/javascript-static.js?r1=1.38&r2=1.39
Hide
Permalink
Nenashev Ilya added a comment -

2 Tim Hunt: http://mootools.net

Show
Nenashev Ilya added a comment - 2 Tim Hunt: http://mootools.net
Hide
Permalink
Martin Dougiamas added a comment -

Sam just worked out that these changes are killing disabledif on Safari ... my +10 to revert them.

Show
Martin Dougiamas added a comment - Sam just worked out that these changes are killing disabledif on Safari ... my +10 to revert them.

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.