r62980 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r62979‎ | r62980 | r62981 >
Date:23:05, 25 February 2010
Author:adam
Status:ok (Comments)
Tags:
Comment:
WikiEditor: partial fix for bug 22347 and updating one image asset for the template editor. FIXME: Needs IE implimentation of the bug fix
Modified paths:
  • /trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php (modified) (history)
  • /trunk/extensions/UsabilityInitiative/images/wikiEditor/templateEditor/dialog-collapsed.png (modified) (history)
  • /trunk/extensions/UsabilityInitiative/js/plugins.combined.js (modified) (history)
  • /trunk/extensions/UsabilityInitiative/js/plugins.combined.min.js (modified) (history)
  • /trunk/extensions/UsabilityInitiative/js/plugins/jquery.wikiEditor.js (modified) (history)

Diff [purge]

Index: trunk/extensions/UsabilityInitiative/UsabilityInitiative.hooks.php
——@@ -72,7 +72,7 @@
7373 array( 'src' => 'js/plugins/jquery.namespaceSelect.js', 'version' => 1 ),
7474 array( 'src' => 'js/plugins/jquery.suggestions.js', 'version' => 7 ),
7575 array( 'src' => 'js/plugins/jquery.textSelection.js', 'version' => 27 ),
76 - array( 'src' => 'js/plugins/jquery.wikiEditor.js', 'version' => 154 ),
 76+ array( 'src' => 'js/plugins/jquery.wikiEditor.js', 'version' => 155 ),
7777 array( 'src' => 'js/plugins/jquery.wikiEditor.highlight.js', 'version' => 34 ),
7878 array( 'src' => 'js/plugins/jquery.wikiEditor.toolbar.js', 'version' => 52 ),
7979 array( 'src' => 'js/plugins/jquery.wikiEditor.dialogs.js', 'version' => 19 ),
——@@ -82,10 +82,10 @@
8383 array( 'src' => 'js/plugins/jquery.wikiEditor.publish.js', 'version' => 3 ),
8484 ),
8585 'combined' => array(
86 - array( 'src' => 'js/plugins.combined.js', 'version' => 290 ),
 86+ array( 'src' => 'js/plugins.combined.js', 'version' => 291 ),
8787 ),
8888 'minified' => array(
89 - array( 'src' => 'js/plugins.combined.min.js', 'version' => 290 ),
 89+ array( 'src' => 'js/plugins.combined.min.js', 'version' => 291 ),
9090 ),
9191 ),
9292 );
Index: trunk/extensions/UsabilityInitiative/images/wikiEditor/templateEditor/dialog-collapsed.png
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Index: trunk/extensions/UsabilityInitiative/js/plugins/jquery.wikiEditor.js
——@@ -1188,12 +1188,19 @@
11891189 'encapsulateSelection': function( options ) {
11901190 var selText = $(this).textSelection( 'getSelection' );
11911191 var selTextArr;
 1192+ var collapseToEnd = false;
11921193 var selectAfter = false;
11931194 var setSelectionTo = null;
11941195 var pre = options.pre, post = options.post;
11951196 if ( !selText ) {
11961197 selText = options.peri;
11971198 selectAfter = true;
 1199+ } else if ( $.wikiEditor.autoMsg( options, 'peri' ) == selText.replace( /\s\s*$/, '' ) ) {
 1200+ // Probably a successive button press
 1201+ // strip any extra white space from selText and set variables we'll need for later
 1202+ selText = selText.replace( /\s\s*$/, '' );
 1203+ collapseToEnd = true;
 1204+ selectAfter = true;
11981205 } else if ( options.replace ) {
11991206 selText = options.peri;
12001207 } else if ( selText.charAt( selText.length - 1 ) == ' ' ) {
——@@ -1209,6 +1216,13 @@
12101217 if ( context.$iframe[0].contentWindow.getSelection ) {
12111218 // Firefox and Opera
12121219 var range = context.$iframe[0].contentWindow.getSelection().getRangeAt( 0 );
 1220+ if ( collapseToEnd ) {
 1221+ // Make sure we're not collapsing ourselves into a BR tag
 1222+ if ( range.endContainer.nodeName == 'BR' ) {
 1223+ range.setEndBefore( range.endContainer );
 1224+ }
 1225+ range.collapse( false );
 1226+ }
12131227 if ( options.ownline ) {
12141228 // We need to figure out if the cursor is at the start or end of a line
12151229 var atStart = false, atEnd = false;
——@@ -1236,7 +1250,6 @@
12371251 // Apparently this happens when splitting text nodes
12381252 atEnd = true;
12391253 }
1240 -
12411254 if ( !atStart ) {
12421255 pre = "\n" + options.pre;
12431256 }
Index: trunk/extensions/UsabilityInitiative/js/plugins.combined.js
——@@ -7625,12 +7625,19 @@
76267626 'encapsulateSelection': function( options ) {
76277627 var selText = $(this).textSelection( 'getSelection' );
76287628 var selTextArr;
 7629+ var collapseToEnd = false;
76297630 var selectAfter = false;
76307631 var setSelectionTo = null;
76317632 var pre = options.pre, post = options.post;
76327633 if ( !selText ) {
76337634 selText = options.peri;
76347635 selectAfter = true;
 7636+ } else if ( $.wikiEditor.autoMsg( options, 'peri' ) == selText.replace( /\s\s*$/, '' ) ) {
 7637+ // Probably a successive button press
 7638+ // strip any extra white space from selText and set variables we'll need for later
 7639+ selText = selText.replace( /\s\s*$/, '' );
 7640+ collapseToEnd = true;
 7641+ selectAfter = true;
76357642 } else if ( options.replace ) {
76367643 selText = options.peri;
76377644 } else if ( selText.charAt( selText.length - 1 ) == ' ' ) {
——@@ -7646,6 +7653,13 @@
76477654 if ( context.$iframe[0].contentWindow.getSelection ) {
76487655 // Firefox and Opera
76497656 var range = context.$iframe[0].contentWindow.getSelection().getRangeAt( 0 );
 7657+ if ( collapseToEnd ) {
 7658+ // Make sure we're not collapsing ourselves into a BR tag
 7659+ if ( range.endContainer.nodeName == 'BR' ) {
 7660+ range.setEndBefore( range.endContainer );
 7661+ }
 7662+ range.collapse( false );
 7663+ }
76507664 if ( options.ownline ) {
76517665 // We need to figure out if the cursor is at the start or end of a line
76527666 var atStart = false, atEnd = false;
——@@ -7673,7 +7687,6 @@
76747688 // Apparently this happens when splitting text nodes
76757689 atEnd = true;
76767690 }
7677 -
76787691 if ( !atStart ) {
76797692 pre = "\n" + options.pre;
76807693 }
Index: trunk/extensions/UsabilityInitiative/js/plugins.combined.min.js
——@@ -516,9 +516,11 @@
517517 context.$textarea.attr('disabled',true);context.$textarea.hide();context.$iframe.show();context.fn.trigger('ready');context.oldHTML=context.oldDelayedHTML=context.$content.html();$(context.$iframe[0].contentWindow.document).bind('keydown',function(event){return context.fn.trigger('keydown',event);}).bind('paste',function(event){return context.fn.trigger('paste',event);}).bind('cut',function(event){return context.fn.trigger('cut',event);}).bind('keyup paste mouseup cut encapsulateSelection',function(event){return context.fn.trigger('change',event);}).delayedBind(250,'keyup paste mouseup cut encapsulateSelection',function(event){context.fn.trigger('delayedChange',event);});});context.$textarea.closest('form').submit(function(){context.$textarea.attr('disabled',false);context.$textarea.val(context.$textarea.textSelection('getContents'));});context.fallbackWindowOnBeforeUnload=window.onbeforeunload;window.onbeforeunload=function(){context.$textarea.val(context.$textarea.textSelection('getContents'));if(context.fallbackWindowOnBeforeUnload){return context.fallbackWindowOnBeforeUnload();}};},'getContents':function(){var html;if($.browser.msie){var $c=$(context.$content.get(0).cloneNode(true));$c.find('p').each(function(){if($(this).html()==''){$(this).replaceWith('<p></p>');}});html=$c.html();}else{html=context.$content.html();}
518518 return context.fn.htmlToText(html);},'getSelection':function(){var retval;if(context.$iframe[0].contentWindow.getSelection){retval=context.$iframe[0].contentWindow.getSelection();if($.browser.opera){if(retval.rangeCount>0){retval=context.fn.htmlToText($('<pre />').append(retval.getRangeAt(0).cloneContents()).html());}else{retval='';}}}else if(context.$iframe[0].contentWindow.document.selection){retval=context.$iframe[0].contentWindow.document.selection.createRange();}
519519 if(typeof retval.text!='undefined'){retval=context.fn.htmlToText(retval.htmlText);}else if(typeof retval.toString!='undefined'){retval=retval.toString();}
520 -return retval;},'encapsulateSelection':function(options){var selText=$(this).textSelection('getSelection');var selTextArr;var selectAfter=false;var setSelectionTo=null;var pre=options.pre,post=options.post;if(!selText){selText=options.peri;selectAfter=true;}else if(options.replace){selText=options.peri;}else if(selText.charAt(selText.length-1)==' '){selText=selText.substring(0,selText.length-1);post+=' ';}
 520+return retval;},'encapsulateSelection':function(options){var selText=$(this).textSelection('getSelection');var selTextArr;var collapseToEnd=false;var selectAfter=false;var setSelectionTo=null;var pre=options.pre,post=options.post;if(!selText){selText=options.peri;selectAfter=true;}else if($.wikiEditor.autoMsg(options,'peri')==selText.replace(/\s\s*$/,'')){selText=selText.replace(/\s\s*$/,'');collapseToEnd=true;selectAfter=true;}else if(options.replace){selText=options.peri;}else if(selText.charAt(selText.length-1)==' '){selText=selText.substring(0,selText.length-1);post+=' ';}
521521 if(options.splitlines){selTextArr=selText.split(/\n/);}
522 -if(context.$iframe[0].contentWindow.getSelection){var range=context.$iframe[0].contentWindow.getSelection().getRangeAt(0);if(options.ownline){var atStart=false,atEnd=false;var body=context.$content.get(0);if(range.startOffset==0){atStart=true;}else if(range.startContainer==body){var n=body.firstChild;for(var i=0;i<range.startOffset-1&&n;i++){n=n.nextSibling;}
 522+if(context.$iframe[0].contentWindow.getSelection){var range=context.$iframe[0].contentWindow.getSelection().getRangeAt(0);if(collapseToEnd){if(range.endContainer.nodeName=='BR'){range.setEndBefore(range.endContainer);}
 523+range.collapse(false);}
 524+if(options.ownline){var atStart=false,atEnd=false;var body=context.$content.get(0);if(range.startOffset==0){atStart=true;}else if(range.startContainer==body){var n=body.firstChild;for(var i=0;i<range.startOffset-1&&n;i++){n=n.nextSibling;}
523525 if(n&&n.nodeName=='BR'){atStart=true;}else{atEnd=true;}}else if(range.startContainer.nodeName=='#text'&&range.startOffset==range.startContainer.nodeValue.length){atEnd=true;}
524526 if(!atStart){pre="\n"+options.pre;}
525527 if(!atEnd){post+="\n";}}

Follow-up revisions

RevisionCommit summaryAuthorDate
r63016Follow up to r62980: Implimenting support for IE and adding more comments exp...adam19:51, 26 February 2010
r63230WikiEditor: Adding more cases for end of line detection in encapsulateSelecti...adam23:15, 3 March 2010

Comments

#Comment by Catrope (talk | contribs)   23:17, 25 February 2010 
+			} else if ( $.wikiEditor.autoMsg( options, 'peri' ) == selText.replace( /\s\s*$/, '' ) ) {

Sorry about our earlier conversation, I didn't know (or ask for) the context. You can just use options.peri here, you don't need autoMsg after all. Also, the regex can be simplified to /\s+$/.

Also, could you maybe explain what collapseToEnd does and why it's set to true when? It's not obvious to me at all (but then again I need sleep).

#Comment by Adammiller~mediawikiwiki (talk | contribs)   01:19, 26 February 2010

collapseToEnd collapses the current selection to the end. It's only set to true when the selected text (sans extra space) matches the default text for the button, indicating that, most likely, the user is just clicking the button multiple times in a row.

All this commit does is makes multiple succedent clicks of the bullet, numbered list, or indent button operate more like you'd expect them to.

I'll try to make that more clear in the comments with my follow up commit.

Status & tagging log