[#MDL-17298] We should not add JavaScript to the YAHOO namespace

Issue Details (XML | Word | Printable)

Key:	MDL-17298
Type:	Sub-task
Status:	Resolved
Resolution:	Fixed
Priority:	Minor
Assignee:	Tim Hunt
Reporter:	Tim Hunt
Votes:	0
Watchers:	1

Operations

Add/Edit UI Mockup to this issue

If you were logged in you would be able to see more operations.

Moodle

MDL-17284

We should not add JavaScript to the YAHOO namespace

Created: 19/Nov/08 04:17 PM Updated: 25/Nov/08 10:21 PM

Return to search

Component/s:

AJAX, Quiz

Affects Version/s:

2.0

Fix Version/s:

2.0

File Attachments:

edit_js_cleanup.patch.txt (6 kB)

Participants:	Olli Savolainen and Tim Hunt
Security Level:	None
Resolved date:	25/Nov/08
Affected Branches:	MOODLE_20_STABLE
Fixed Branches:	MOODLE_20_STABLE

Description

« Hide

Sorry, this one is quite big. I don't know why I did not notice it before.

We really should not be adding our code to the YAHOO library namespace. I would change

YAHOO.cats.container -> question_bank
YAHOO.quiz.container -> quiz_editor

or something like that.

<div id="module" is not very descriptive. Wouldn't something like id="questionbankcontents" be better?

Do you really want to initialise on load, would onDomReady work better?

All

Comments

Change History

Version Control

Sort Order:

Olli Savolainen committed 1 file to 'Moodle CVS' - 23/Nov/08 02:20 AM

quiz editing: ~~MDL-17298~~ use onDOMReady instead of onload.

In the previous commit to this file (1.2) I accidentally tried to do this already, but did it wrong, so javascript dialogs did not work.

MODIFY

mod/quiz/edit.js

Rev. 1.3

(+2 -1 lines)

Olli Savolainen committed 1 file to 'Moodle CVS' - 23/Nov/08 02:26 AM

quiz editing: ~~MDL-17298~~ removed debugging code forgotten there in the previous revision

MODIFY

mod/quiz/edit.js

Rev. 1.4

(+0 -1 lines)

[ Permalink | « Hide ]

Olli Savolainen added a comment - 23/Nov/08 02:41 AM

Changed it to OnDOMReady.

http://www.zachleat.com/web/2007/08/09/yui-code-review-yahoonamespace/

From this I understand that the YAHOO namespace is indeed intended for other uses than just Yahoo!'s. However, I am not sure if that code applies otherwise to be used in a namespace.

So I am not sure what exactly to fix about the namespaces. If you still think it is necessary, please fix it? Thanks.

Olli Savolainen made changes - 23/Nov/08 03:06 AM

Field	Original Value	New Value
Assignee	Olli Savolainen [ pilpi ]	Tim Hunt [ timhunt ]

[ Permalink | « Hide ]

Tim Hunt added a comment - 23/Nov/08 08:13 PM

OK, I'll deal with this.

(By the way, the blog post you link to is just about what is technically possible, not what is recommended. Indeed, another post from the same blog: http://www.zachleat.com/web/2007/08/28/namespacing-outside-of-the-yahoo-namespace/)

[ Permalink | « Hide ]

Tim Hunt added a comment - 23/Nov/08 11:01 PM

Work in progress patch. Does not work. I just want to be able to pick this up tomorrow at work.

Tim Hunt made changes - 23/Nov/08 11:01 PM

Attachment

edit_js_cleanup.patch.txt [ 15716 ]

[ Permalink | « Hide ]

Olli Savolainen added a comment - 24/Nov/08 08:06 PM

Alright then. Thanks.

Though I do wonder about the following quote on the page you referred to, and about why didn't the yahoo folks then think of that, and where is the official documentation for YAHOO.namespace() (and frankly I also wonder why the snow that fell here yesterday had to melt last night), even though I don't see how two namespaces could coexist with the same name:

"I'm glad you're enjoying using YUITest. I think your point about namespacing is well-taken, but the YAHOO.namespace() function is non-destructive. If you were already using YAHOO.tool, then YAHOO.namespace() won't overwrite the existing one so all of your functionality will be safe."

[ Permalink | « Hide ]

Tim Hunt added a comment - 25/Nov/08 11:43 AM

Well, to my mind, the whole point of namespaces is to completely avoid naming conflicts. If we keep stuff starting with YAHOO == Yahoo's code, and put Moodle code in other namespaces, then we can be sure that there will never be a name collisions caused by anything YAHOO does. Anyway, should be easy to finish fixing when I get a moment.

tjhunt committed 2 files to 'Moodle CVS' - 25/Nov/08 09:31 PM

quiz editing: ~~MDL-17298~~ We should not add JavaScript to the YAHOO namespace

Also, use the newer methods for including required JavaScript.

MODIFY	mod/quiz/edit.php		Rev. 1.130		(+18 -30 lines)
MODIFY	mod/quiz/edit.js		Rev. 1.5		(+12 -15 lines)

[ Permalink | « Hide ]

Tim Hunt added a comment - 25/Nov/08 09:32 PM

Right, I think I have done this, and having tested, I don't think I have broken anything. Could you give it a quick test please Olli, since you know better than me how it is supposed to work. Thanks

Tim Hunt made changes - 25/Nov/08 10:18 PM

Resolution		Fixed [ 1 ]
Status	Open [ 1 ]	Resolved [ 5 ]

[ Permalink | « Hide ]

Olli Savolainen added a comment - 25/Nov/08 10:21 PM

The two dialogs seem to work OK. Thank you.

All content on this web site is made available under the GNU General Public License, unless otherwise stated.