From df0fa5159cff309a169a316d04c59719b141e4f0 Mon Sep 17 00:00:00 2001 From: Rob Moen Date: Tue, 24 Jul 2012 10:42:29 -0700 Subject: [PATCH] Bug 33088 - VisualEditor: Editing a part of text of a link doesn't work (or this shouldn't be allowed) -Revised method for for returning all link annotations in a selection. Now properly clearning all selected links. -Trimming whitespace from selection -Modifying selection if it doesn't contain annotated range -Disabled link creation only if target is blank. This allows Existing link text to be modified while having the same target. Change-Id: I7255dcf1c88fa1cd6e7edbc3baa82cd4c72a95d1 --- modules/ve/dm/ve.dm.Document.js | 31 ++++++++- .../ve/ui/inspectors/ve.ui.LinkInspector.js | 65 +++++++++++-------- 2 files changed, 66 insertions(+), 30 deletions(-) diff --git a/modules/ve/dm/ve.dm.Document.js b/modules/ve/dm/ve.dm.Document.js index 5e58cfbaa9..010b0995f6 100644 --- a/modules/ve/dm/ve.dm.Document.js +++ b/modules/ve/dm/ve.dm.Document.js @@ -502,7 +502,7 @@ ve.dm.Document.prototype.getAnnotationsFromOffset = function( offset ) { */ ve.dm.Document.prototype.offsetContainsAnnotation = function ( offset, annotation ) { var annotations = this.getAnnotationsFromOffset( offset ); - for (var a in annotations) { + for ( var a in annotations ) { if ( ve.compareObjects( annotations[a], annotation ) ){ return true; } @@ -531,7 +531,34 @@ ve.dm.Document.prototype.getAnnotatedRangeFromOffset = function ( offset, annota } } while ( end < this.data.length ) { - if ( this.offsetContainsAnnotation(end, annotation ) === false ) { + if ( this.offsetContainsAnnotation( end, annotation ) === false ) { + break; + } + end++; + } + return new ve.Range( start, end ); +}; + +/** + * Gets the range of an annotation found in the selection range. + * + * @param {Integer} offset Offset to begin looking forward and backward from + * @param {Object} annotation Annotation to test for coverage with + * @returns {ve.Range|null} Range of content covered by annotation, or a copy of the range. + */ +ve.dm.Document.prototype.getAnnotatedRangeFromSelection = function( range, annotation ) { + var start = range.start, + end = range.end; + + while ( start > 0 ) { + start--; + if ( this.offsetContainsAnnotation( start, annotation ) === false ) { + start++; + break; + } + } + while ( end < this.data.length ) { + if ( this.offsetContainsAnnotation( end, annotation ) === false ) { break; } end++; diff --git a/modules/ve/ui/inspectors/ve.ui.LinkInspector.js b/modules/ve/ui/inspectors/ve.ui.LinkInspector.js index 7812910d8f..d846d364d2 100644 --- a/modules/ve/ui/inspectors/ve.ui.LinkInspector.js +++ b/modules/ve/ui/inspectors/ve.ui.LinkInspector.js @@ -38,8 +38,8 @@ ve.ui.LinkInspector = function( toolbar, context ) { var hash, surfaceModel = inspector.context.getSurfaceView().getModel(), - annotations = inspector.getSelectedLinkAnnotations(); - // If link annotation exists, clear it. + annotations = inspector.getAllLinkAnnotationsFromSelection(); + // Clear all link annotations. for ( hash in annotations ) { surfaceModel.annotate( 'clear', annotations[hash] ); } @@ -48,7 +48,7 @@ ve.ui.LinkInspector = function( toolbar, context ) { } ); this.$locationInput.bind( 'mousedown keydown cut paste', function() { setTimeout( function() { - if ( inspector.$locationInput.val() !== inspector.initialValue ) { + if ( inspector.$locationInput.val() !== '' ) { inspector.$acceptButton.removeClass( 'es-inspector-button-disabled' ); } else { inspector.$acceptButton.addClass( 'es-inspector-button-disabled' ); @@ -59,23 +59,24 @@ ve.ui.LinkInspector = function( toolbar, context ) { /* Methods */ -ve.ui.LinkInspector.prototype.getSelectedLinkAnnotations = function(){ +ve.ui.LinkInspector.prototype.getAllLinkAnnotationsFromSelection = function() { var surfaceView = this.context.getSurfaceView(), surfaceModel = surfaceView.getModel(), documentModel = surfaceModel.getDocument(), - data = documentModel.getData( surfaceModel.getSelection() ); + annotations, + linkAnnotations = {}; - if ( data.length ) { - if ( ve.isPlainObject( data[0][1] ) ) { - return ve.dm.Document.getMatchingAnnotations( data[0][1], /link\/.*/ ); + annotations = documentModel.getAnnotationsFromRange( surfaceModel.getSelection(), true ); + linkAnnotations = ve.dm.Document.getMatchingAnnotations ( annotations, /link\/.*/ ); + if ( !ve.isEmptyObject( linkAnnotations ) ) { + return linkAnnotations; } - } - return {}; + + return null; }; -ve.ui.LinkInspector.prototype.getAnnotationFromSelection = function() { - var hash, - annotations = this.getSelectedLinkAnnotations(); +ve.ui.LinkInspector.prototype.getFirstLinkAnnotation = function( annotations ) { + var hash; for ( hash in annotations ) { // Use the first one with a recognized type (there should only be one, but this is just in case) if ( annotations[hash].type === 'link/wikiLink' || annotations[hash].type === 'link/extLink' ) { @@ -113,27 +114,34 @@ ve.ui.LinkInspector.prototype.prepareOpen = function() { var surfaceView = this.context.getSurfaceView(), surfaceModel = surfaceView.getModel(), doc = surfaceModel.getDocument(), - annotation = this.getAnnotationFromSelection(), + annotations = this.getAllLinkAnnotationsFromSelection(), + annotation = this.getFirstLinkAnnotation( annotations ), selection = surfaceModel.getSelection(), + annotatedRange, newSelection; - if ( annotation !== null ) { - // Ensure that the entire annotated range is selected - newSelection = doc.getAnnotatedRangeFromOffset( selection.start, annotation ); - // Apply selection direction to new selection - if ( selection.from > selection.start ) { - newSelection.flip(); - } - } else { - // No annotation, trim outer space from range - newSelection = doc.trimOuterSpaceFromRange( selection ); - } + // Trim outer space from range if any. + newSelection = doc.trimOuterSpaceFromRange( selection ); + if ( annotation !== null ) { + annotatedRange = doc.getAnnotatedRangeFromSelection( newSelection, annotation ); + + // Adjust selection if it does not contain the annotated range + if ( selection.start > annotatedRange.start || + selection.end < annotatedRange.end + ) { + newSelection = annotatedRange; + // if selected from right to left + if ( selection.from > selection.start ) { + newSelection.flip(); + } + } + } surfaceModel.change( null, newSelection ); }; ve.ui.LinkInspector.prototype.onOpen = function() { - var annotation = this.getAnnotationFromSelection(), + var annotation = this.getFirstLinkAnnotation( this.getAllLinkAnnotationsFromSelection() ), initialValue = ''; if ( annotation === null ) { this.$locationInput.val( this.getSelectionText() ); @@ -165,11 +173,11 @@ ve.ui.LinkInspector.prototype.onOpen = function() { ve.ui.LinkInspector.prototype.onClose = function( accept ) { var surfaceView = this.context.getSurfaceView(), surfaceModel = surfaceView.getModel(), - annotations = this.getSelectedLinkAnnotations(), + annotations = this.getAllLinkAnnotationsFromSelection(), target = this.$locationInput.val(), hash, annotation; if ( accept ) { - if ( target === this.initialValue || !target ) { + if ( !target ) { return; } // Clear link annotation if it exists @@ -177,6 +185,7 @@ ve.ui.LinkInspector.prototype.onClose = function( accept ) { surfaceModel.annotate( 'clear', annotations[hash] ); } surfaceModel.annotate( 'set', ve.ui.LinkInspector.getAnnotationForTarget( target ) ); + } // Restore focus surfaceView.getDocument().getDocumentNode().$.focus();