r73512 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r73511‎ | r73512 | r73513 >
Date:08:28, 22 September 2010
Author:maxsem
Status:reverted (Comments)
Tags:
Comment:
Fixed a JS error in EditWarning
Modified paths:
  • /trunk/extensions/Vector/modules/ext.vector.editWarning.js (modified) (history)

Diff [purge]

Index: trunk/extensions/Vector/modules/ext.vector.editWarning.js
——@@ -3,7 +3,7 @@
44 */
55 $(document).ready( function() {
66 // Check if EditWarning is enabled and if we need it
7 - if ( $( '#wpTextbox1' ).size() == 0 ) {
 7+ if ( $( '#wpTextbox1' ) == null ) {
88 return true;
99 }
1010 // Get the original values of some form elements

Follow-up revisions

RevisionCommit summaryAuthorDate
r74265Follow-up r73512: per Roan's comment wrapped the whole thing in closure insteadmaxsem18:22, 4 October 2010

Comments

#Comment by Catrope (talk | contribs)   20:29, 1 October 2010

That doesn't look right. The jQuery constructor should never return null: it promises to always return a jQuery object, which might be empty. Something weird is going on here; it's probably caused by the code not being wrapped in (function($) { ... })(jQuery);.

#Comment by MaxSem (talk | contribs)   21:00, 1 October 2010

It was null for me in the wild.

#Comment by Catrope (talk | contribs)   21:01, 1 October 2010

That's probably because something called jQuery.noConflict(); , which detaches $ from jQuery and makes it do weird things. This is why the module should be wrapped in a closure, so $ guaranteed to be an alias for jQuery.

#Comment by MaxSem (talk | contribs)   17:04, 2 October 2010

Will do, however, should 

//Global storage of fallback for onbeforeunload hook
var fallbackWindowOnBeforeUnload = null;

be left in global scope?

#Comment by Catrope (talk | contribs)   14:28, 3 October 2010

Probably not, no.

#Comment by MaxSem (talk | contribs)   18:23, 4 October 2010

Done in r74265.

Status & tagging log