r106249 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r106248‎ \| r106249 \| r106250 >
Date:	21:58, 14 December 2011
Author:	neilk
Status:	ok
Tags:
Comment:	MFT r106123, r106124, r106127, r106157, r106224, r106230 misc fixes to commonly-reported bugs in VisualEditor, particularly input methods and links
Modified paths:	/branches/wmf/1.18wmf1/extensions/VisualEditor (modified) (history) /branches/wmf/1.18wmf1/extensions/VisualEditor/VisualEditor.php (modified) (history) /branches/wmf/1.18wmf1/extensions/VisualEditor/modules/es/models/es.DocumentModel.js (modified) (history) /branches/wmf/1.18wmf1/extensions/VisualEditor/modules/es/tools/es.AnnotationButtonTool.js (modified) (history) /branches/wmf/1.18wmf1/extensions/VisualEditor/modules/es/views/es.SurfaceView.js (modified) (history) /branches/wmf/1.18wmf1/extensions/VisualEditor/modules/sandbox/base.php (modified) (history) /branches/wmf/1.18wmf1/extensions/VisualEditor/modules/sandbox/special.js (modified) (history)

Diff [purge]

Index: branches/wmf/1.18wmf1/extensions/VisualEditor/VisualEditor.php
—	—	@@ -57,6 +57,7 @@
58	58	'dependencies' => array(
59	59	'ext.visualEditor.sandbox',
60	60	'mediawiki.feedback',
	61	+ 'mediawiki.Uri',
61	62	)
62	63	),
63	64	'ext.visualEditor.sandbox' => $wgVisualEditorResourceTemplate + array(
Index: branches/wmf/1.18wmf1/extensions/VisualEditor/modules/es/tools/es.AnnotationButtonTool.js
—	—	@@ -26,8 +26,13 @@
27	27	if ( surfaceView.getModel().getSelection().getLength() ) {
28	28	this.toolbar.getSurfaceView().getContextView().openInspector( this.inspector );
29	29	} else {
30		~~- if ( !this.active ) {~~
31		~~- surfaceView.annotate( 'set', this.annotation );~~
	30	+ if ( this.active ) {
	31	+ var surfaceModel = surfaceView.getModel(),
	32	+ documentModel = surfaceModel.getDocument(),
	33	+ selection = surfaceModel.getSelection(),
	34	+ range = documentModel.getAnnotationBoundaries( selection.from, this.annotation, true );
	35	+ surfaceModel.select( range );
	36	+ this.toolbar.getSurfaceView().getContextView().openInspector( this.inspector );
32	37	}
33	38	}
34	39	} else {
Index: branches/wmf/1.18wmf1/extensions/VisualEditor/modules/es/models/es.DocumentModel.js
—	—	@@ -682,11 +682,11 @@
683	683	* @param {Object} annotation Annotation to test for coverage with
684	684	* @returns {es.Range\|null} Range of content covered by annotation, or null if offset is not covered
685	685	*/
686		~~-es.DocumentModel.prototype.getAnnotationBoundaries = function( offset, annotation ) {~~
	686	+es.DocumentModel.prototype.getAnnotationBoundaries = function( offset, annotation, typeOnly ) {
687	687	if ( annotation.hash === undefined ) {
688	688	annotation.hash = es.DocumentModel.getHash( annotation );
689	689	}
690		~~- if ( es.DocumentModel.getIndexOfAnnotation( this.data[offset], annotation ) === -1 ) {~~
	690	+ if ( es.DocumentModel.getIndexOfAnnotation( this.data[offset], annotation, typeOnly ) === -1 ) {
691	691	return null;
692	692	}
693	693	var start = offset,
—	—	@@ -694,13 +694,13 @@
695	695	item;
696	696	while ( start > 0 ) {
697	697	start--;
698		~~- if ( es.DocumentModel.getIndexOfAnnotation( this.data[start], annotation ) === -1 ) {~~
	698	+ if ( es.DocumentModel.getIndexOfAnnotation( this.data[start], annotation, typeOnly ) === -1 ) {
699	699	start++;
700	700	break;
701	701	}
702	702	}
703	703	while ( end < this.data.length ) {
704		~~- if ( es.DocumentModel.getIndexOfAnnotation( this.data[end], annotation ) === -1 ) {~~
	704	+ if ( es.DocumentModel.getIndexOfAnnotation( this.data[end], annotation, typeOnly ) === -1 ) {
705	705	break;
706	706	}
707	707	end++;
Index: branches/wmf/1.18wmf1/extensions/VisualEditor/modules/es/views/es.SurfaceView.js
—	—	@@ -23,7 +23,7 @@
24	24	this.$ = $container
25	25	.addClass( 'es-surfaceView' )
26	26	.append( this.documentView.$ );
27		~~- this.$input = $( '<textarea class="es-surfaceView-textarea" />' )~~
	27	+ this.$input = $( '<textarea class="es-surfaceView-textarea" autocapitalize="off" />' )
28	28	.appendTo( 'body' );
29	29	this.$cursor = $( '<div class="es-surfaceView-cursor"></div>' )
30	30	.appendTo( 'body' );
—	—	@@ -132,29 +132,34 @@
133	133	}, 0 );
134	134	}
135	135	} );
136		~~- $window.resize( function() {~~
137		~~- // Re-render when resizing horizontally~~
138		~~- // TODO: Instead of re-rendering on every single 'resize' event wait till user is done with~~
139		~~- // resizing - can be implemented with setTimeout~~
140		~~- _this.hideCursor();~~
141		~~- _this.dimensions.height = $window.height();~~
142		~~- // XXX: This is a dirty hack!~~
143		~~- _this.dimensions.toolbarHeight = $( '#es-toolbar' ).height();~~
144		~~- var width = _this.$.width();~~
145		~~- if ( _this.dimensions.width !== width ) {~~
146		~~- _this.dimensions.width = width;~~
147		~~- _this.documentView.renderContent();~~
148		~~- _this.emitUpdate( 25 );~~
149		~~- }~~
150		~~- } );~~
151		~~- $window.scroll( function() {~~
152		~~- _this.dimensions.scrollTop = $window.scrollTop();~~
153		~~- if ( _this.contextView ) {~~
154		~~- if ( _this.currentSelection.getLength() && !_this.mouse.selectingMode ) {~~
155		~~- _this.contextView.set();~~
156		~~- } else {~~
157		~~- _this.contextView.clear();~~
	136	+ $window.bind( {
	137	+ 'resize': function() {
	138	+ // Re-render when resizing horizontally
	139	+ // TODO: Instead of re-rendering on every single 'resize' event wait till user is done
	140	+ // with resizing - can be implemented with setTimeout
	141	+ _this.hideCursor();
	142	+ _this.dimensions.height = $window.height();
	143	+ // XXX: This is a dirty hack!
	144	+ _this.dimensions.toolbarHeight = $( '#es-toolbar' ).height();
	145	+ var width = _this.$.width();
	146	+ if ( _this.dimensions.width !== width ) {
	147	+ _this.dimensions.width = width;
	148	+ _this.documentView.renderContent();
	149	+ _this.emitUpdate( 25 );
158	150	}
	151	+ },
	152	+ 'scroll': function() {
	153	+ _this.dimensions.scrollTop = $window.scrollTop();
	154	+ if ( _this.contextView ) {
	155	+ if ( _this.currentSelection.getLength() && !_this.mouse.selectingMode ) {
	156	+ _this.contextView.set();
	157	+ } else {
	158	+ _this.contextView.clear();
	159	+ }
	160	+ }
	161	+ },
	162	+ 'blur': function() {
	163	+ _this.keyboard.keys.shift = false;
159	164	}
160	165	} );
161	166
—	—	@@ -626,6 +631,14 @@
627	632	case 75:
628	633	if ( this.currentSelection.getLength() ) {
629	634	this.contextView.openInspector( 'link' );
	635	+ } else {
	636	+ var range = this.model.getDocument().getAnnotationBoundaries(
	637	+ this.currentSelection.from, { 'type': 'link/internal' }, true
	638	+ );
	639	+ if ( range ) {
	640	+ this.model.select( range );
	641	+ this.contextView.openInspector( 'link' );
	642	+ }
630	643	}
631	644	return false;
632	645	}
Index: branches/wmf/1.18wmf1/extensions/VisualEditor/modules/sandbox/base.php
—	—	@@ -33,21 +33,21 @@
34	34	<div class="es-help-shortcuts-title">Clipboard</div>
35	35	<div class="es-help-shortcut">
36	36	<span class="es-help-keys">
37		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	37	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
38	38	<span class="es-help-key">C</span>
39	39	</span>
40	40	Copy selected text
41	41	</div>
42	42	<div class="es-help-shortcut">
43	43	<span class="es-help-keys">
44		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	44	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
45	45	<span class="es-help-key">X</span>
46	46	</span>
47	47	Cut selected text
48	48	</div>
49	49	<div class="es-help-shortcut">
50	50	<span class="es-help-keys">
51		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	51	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
52	52	<span class="es-help-key">V</span>
53	53	</span>
54	54	Paste text at the cursor
—	—	@@ -55,21 +55,21 @@
56	56	<div class="es-help-shortcuts-title">History navigation</div>
57	57	<div class="es-help-shortcut">
58	58	<span class="es-help-keys">
59		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	59	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
60	60	<span class="es-help-key">Z</span>
61	61	</span>
62	62	Undo
63	63	</div>
64	64	<div class="es-help-shortcut">
65	65	<span class="es-help-keys">
66		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	66	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
67	67	<span class="es-help-key">Y</span>
68	68	</span>
69	69	Redo
70	70	</div>
71	71	<div class="es-help-shortcut">
72	72	<span class="es-help-keys">
73		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	73	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
74	74	<span class="es-help-key">⇧</span> +
75	75	<span class="es-help-key">Z</span>
76	76	</span>
—	—	@@ -78,21 +78,21 @@
79	79	<div class="es-help-shortcuts-title">Formatting</div>
80	80	<div class="es-help-shortcut">
81	81	<span class="es-help-keys">
82		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	82	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
83	83	<span class="es-help-key">B</span>
84	84	</span>
85	85	Make selected text bold
86	86	</div>
87	87	<div class="es-help-shortcut">
88	88	<span class="es-help-keys">
89		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	89	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
90	90	<span class="es-help-key">I</span>
91	91	</span>
92	92	Make selected text italic
93	93	</div>
94	94	<div class="es-help-shortcut">
95	95	<span class="es-help-keys">
96		~~- <span class="es-help-key">Ctl <span class="es-help-key-or">or</span> ⌘</span> +~~
	96	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌘</span> +
97	97	<span class="es-help-key">K</span>
98	98	</span>
99	99	Make selected text a link
—	—	@@ -107,14 +107,14 @@
108	108	</div>
109	109	<div class="es-help-shortcut">
110	110	<span class="es-help-keys">
111		~~- <span class="es-help-key">Alt</span> +~~
	111	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌥</span> +
112	112	<span class="es-help-key">Arrow</span>
113	113	</span>
114	114	Move cursor by words or blocks
115	115	</div>
116	116	<div class="es-help-shortcut">
117	117	<span class="es-help-keys">
118		~~- <span class="es-help-key">Alt</span> +~~
	118	+ <span class="es-help-key">Ctrl <span class="es-help-key-or">or</span> ⌥</span> +
119	119	<span class="es-help-key">⇧</span> +
120	120	<span class="es-help-key">Arrow</span>
121	121	</span>
Index: branches/wmf/1.18wmf1/extensions/VisualEditor/modules/sandbox/special.js
—	—	@@ -6,7 +6,9 @@
7	7
8	8	var feedback = new mw.Feedback( {
9	9	'title': new mw.Title( 'Visual editor/Feedback' ),
10		~~- 'dialogTitleMessageKey': 'visualeditor-feedback-dialog-title'~~
	10	+ 'dialogTitleMessageKey': 'visualeditor-feedback-dialog-title',
	11	+ 'bugsLink': new mw.Uri( '//bugzilla.wikimedia.org/enter_bug.cgi?product=MediaWiki%20extensions&component=VisualEditor' ),
	12	+ 'bugsListLink': new mw.Uri( '//bugzilla.wikimedia.org/buglist.cgi?query_format=advanced&component=VisualEditor&resolution=---&resolution=LATER&resolution=DUPLICATE&product=MediaWiki%20extensions' )
11	13	} );
12	14
13	15	$feedbackLink = $( '<a></a>' )
Property changes on: branches/wmf/1.18wmf1/extensions/VisualEditor
___________________________________________________________________
Added: svn:mergeinfo
14	16	Merged /branches/new-installer/phase3/extensions/VisualEditor:r43664-66004
15	17	Merged /branches/REL1_15/phase3/extensions/VisualEditor:r51646
16	18	Merged /branches/REL1_18/extensions/VisualEditor:r101758,103190
17	19	Merged /branches/REL1_17/phase3/extensions/VisualEditor:r81445,81448
18	20	Merged /trunk/extensions/VisualEditor:r99592,99653,100092,100419,100686,100692,100699,103669,104337,104736,104863,106123-106124,106127,106157,106224,106230
19	21	Merged /branches/sqlite/extensions/VisualEditor:r58211-58321
20	22	Merged /trunk/phase3/extensions/VisualEditor:r92580,92634,92713,92762,92765,92791,92854,92884,92886-92887,92894,92898,92907,92932,92958,93141,93149,93151,93233-93234,93258,93266,93303,93516-93518,93520,93818-93822,93847,93858,93891,93935-93936,94058,94062,94068,94107,94155,94235,94277,94346,94372,94422,94425,94444,94448,94456,94498,94517,94601,94630,94728,94738,94825,94862,94995-94997,95023,95042,95072-95073,95155,95327,95332,95410,95422,95426,95442,95468,95601,95812,98578,98598,98656

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r106123	- Added auto-link selection when opening the link editor without selecting an...	tparscal	23:12, 13 December 2011
r106124	Fixes issue with r106123 where creating new links wasn't working anymore with...	tparscal	23:15, 13 December 2011
r106127	Added blur handler for window which resets the shift key tracker	tparscal	23:22, 13 December 2011
r106157	add buglist ang bug reporting links to feedback form	neilk	01:32, 14 December 2011
r106224	Added autocapitalize="off" attribute to text area input so IOS doesn't capita...	tparscal	18:54, 14 December 2011
r106230	Fixed help panel content where we claimed the alt key was to be used for word...	tparscal	19:15, 14 December 2011

Status & tagging log

11:01, 16 December 2011 Nikerabbit (talk | contribs) changed the status of r106249 [removed: new added: ok]