Esanders has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/324777

Change subject: [BREAKING CHANGE] Parse selection before applying source tools
......................................................................

[BREAKING CHANGE] Parse selection before applying source tools

If I select '<source syntax>foo</source>' and apply a link or
trigger any other tool, it should parse that selection into
DM, instead of treating is a plain text, otherwise it will
get escaped.

Change-Id: Iaa41c49e2f8e28af28a8a64cb6915f705fa35e8c
---
M src/dm/ve.dm.SourceSurfaceFragment.js
M src/ui/actions/ve.ui.WindowAction.js
2 files changed, 112 insertions(+), 141 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor 
refs/changes/77/324777/1

diff --git a/src/dm/ve.dm.SourceSurfaceFragment.js 
b/src/dm/ve.dm.SourceSurfaceFragment.js
index 72f39da..a4b4b77 100644
--- a/src/dm/ve.dm.SourceSurfaceFragment.js
+++ b/src/dm/ve.dm.SourceSurfaceFragment.js
@@ -26,30 +26,21 @@
  * @inheritdoc
  */
 ve.dm.SourceSurfaceFragment.prototype.annotateContent = function () {
-       var fragment, tempDocument, rangeInDocument, tempSurfaceModel,
-               originalDocument = this.getDocument(),
-               coveringRange = this.getSelection().getCoveringRange();
+       var tempFragment, tempSurfaceModel,
+               args = arguments,
+               fragment = this,
+               text = this.getText( true );
 
-       if ( !coveringRange.isCollapsed() ) {
-               tempDocument = originalDocument.shallowCloneFromRange( 
coveringRange );
-               rangeInDocument = tempDocument.originalRange;
-       } else {
-               tempDocument = new ve.dm.Document(
-                       [
-                               { type: 'paragraph', internal: { generated: 
'wrapper' } }, { type: '/paragraph' },
-                               { type: 'internalList' }, { type: 
'/internalList' }
-                       ],
-                       null, null, null, null,
-                       originalDocument.getLang(),
-                       originalDocument.getDir()
+       this.convertFromSource( text ).then( function ( selectionDocument ) {
+               tempSurfaceModel = new ve.dm.Surface( selectionDocument );
+               tempFragment = tempSurfaceModel.getLinearFragment(
+                       // TODO: Find content offsets
+                       new ve.Range( 0, 
selectionDocument.getInternalList().getListNode().getOuterRange().start )
                );
-               rangeInDocument = new ve.Range( 1 );
-       }
-       tempSurfaceModel = new ve.dm.Surface( tempDocument );
-       fragment = tempSurfaceModel.getLinearFragment( rangeInDocument );
-       fragment.annotateContent.apply( fragment, arguments );
+               tempFragment.annotateContent.apply( tempFragment, args );
 
-       this.insertDocument( fragment.getDocument() );
+               fragment.insertDocument( tempFragment.getDocument() );
+       } );
 
        return this;
 };
@@ -89,7 +80,7 @@
                return 
ve.dm.SourceSurfaceFragment.super.prototype.insertContent.call( this, 
doc.data.getDataSlice( newDocRange ) );
        }
 
-       /* conversionPromise = */ this.convertDocument( doc )
+       this.convertToSource( doc )
                .done( function ( source ) {
                        fragment.removeContent();
 
@@ -106,57 +97,7 @@
 };
 
 /**
- * @inheritdoc
- */
-ve.dm.SourceSurfaceFragment.prototype.wrapAllNodes = function ( wrapOuter, 
wrapEach ) {
-       var i, node, nodes,
-               content,
-               range = this.getSelection().getCoveringRange();
-
-       if ( !range ) {
-               return this;
-       }
-
-       function getOpening( element ) {
-               element.internal = {
-                       whitespace: [ '\n', '\n', '\n', '\n' ]
-               };
-               return element;
-       }
-
-       function getClosing( element ) {
-               return { type: '/' + element.type };
-       }
-
-       if ( !Array.isArray( wrapOuter ) ) {
-               wrapOuter = [ wrapOuter ];
-       }
-
-       wrapEach = wrapEach || [];
-
-       if ( !Array.isArray( wrapEach ) ) {
-               wrapEach = [ wrapEach ];
-       }
-
-       nodes = this.getSelectedLeafNodes();
-
-       content = wrapOuter.map( getOpening );
-       for ( i = 0; i < nodes.length; i++ ) {
-               node = nodes[ i ];
-               content = content
-                       .concat( wrapEach.map( getOpening ) )
-                       .concat( this.getSurface().getLinearFragment( 
node.getRange() ).getText().split( '' ) )
-                       .concat( wrapEach.reverse().map( getClosing ) );
-       }
-       content = content.concat( wrapOuter.reverse().map( getClosing ) );
-
-       this.insertContent( content );
-
-       return this;
-};
-
-/**
- * Convert sub document to source text
+ * Convert model slice to source text
  *
  * The default implementation converts to HTML synchronously.
  *
@@ -166,7 +107,7 @@
  * @param {ve.dm.Document} doc Document
  * @return {jQuery.Promise} Promise with resolves with source, or rejects
  */
-ve.dm.SourceSurfaceFragment.prototype.convertDocument = function ( doc ) {
+ve.dm.SourceSurfaceFragment.prototype.convertToSource = function ( doc ) {
        if ( !doc.data.hasContent() ) {
                return $.Deferred().reject().promise();
        } else {
@@ -177,3 +118,38 @@
                ).promise();
        }
 };
+
+/**
+ * Convert source text to model slice
+ *
+ * The default implementation converts from HTML synchronously.
+ *
+ * If the conversion is asynchornous it should lock the surface
+ * until complete.
+ *
+ * @param {string} source Source text
+ * @return {jQuery.Promise} Promise with resolves with source
+ */
+ve.dm.SourceSurfaceFragment.prototype.convertFromSource = function ( source ) {
+       var lang = this.getDocument().getLang(),
+               dir = this.getDocument().getDir();
+
+       if ( !source ) {
+               return $.Deferred().resolve(
+                       new ve.dm.Document(
+                               [
+                                       { type: 'paragraph', internal: { 
generated: 'wrapper' } }, { type: '/paragraph' },
+                                       { type: 'internalList' }, { type: 
'/internalList' }
+                               ],
+                               null, null, null, null,
+                               lang, dir
+                       )
+               ).promise();
+       } else {
+               return $.Deferred().resolve(
+                       ve.dm.converter.getModelFromDom(
+                               ve.createDocumentFromHtml( source, { lang: 
lang, dir: dir } )
+                       )
+               ).promise();
+       }
+};
diff --git a/src/ui/actions/ve.ui.WindowAction.js 
b/src/ui/actions/ve.ui.WindowAction.js
index 7ec37cd..1d652f2 100644
--- a/src/ui/actions/ve.ui.WindowAction.js
+++ b/src/ui/actions/ve.ui.WindowAction.js
@@ -45,8 +45,8 @@
  * @return {boolean} Action was executed
  */
 ve.ui.WindowAction.prototype.open = function ( name, data, action ) {
-       var currentInspector, inspectorWindowManager,
-               originalFragment, originalDocument, coveringRange, 
rangeInDocument, tempDocument, tempSurfaceModel,
+       var currentInspector, inspectorWindowManager, fragmentPromise,
+               originalFragment, text,
                windowType = this.getWindowType( name ),
                windowManager = windowType && this.getWindowManager( windowType 
),
                currentWindow = windowManager.getCurrentWindow(),
@@ -64,81 +64,76 @@
        }
 
        if ( sourceMode ) {
-               // TODO: This duplicates code in 
SourceSurfaceFragment#annotateContent
+               text = fragment.getText( true );
                originalFragment = fragment;
-               originalDocument = originalFragment.getDocument();
-               coveringRange = 
originalFragment.getSelection().getCoveringRange();
-               if ( coveringRange && !coveringRange.isCollapsed() ) {
-                       tempDocument = 
surface.getModel().getDocument().shallowCloneFromRange( coveringRange );
-                       rangeInDocument = tempDocument.originalRange;
-               } else {
-                       tempDocument = new ve.dm.Document(
-                               [
-                                       { type: 'paragraph', internal: { 
generated: 'wrapper' } }, { type: '/paragraph' },
-                                       { type: 'internalList' }, { type: 
'/internalList' }
-                               ],
-                               null, null, null, null,
-                               originalDocument.getLang(),
-                               originalDocument.getDir()
-                       );
-                       rangeInDocument = new ve.Range( 1 );
-               }
-               tempSurfaceModel = new ve.dm.Surface( tempDocument );
-               fragment = tempSurfaceModel.getLinearFragment( rangeInDocument 
);
+
+               fragmentPromise = fragment.convertFromSource( text ).then( 
function ( selectionDocument ) {
+                       var tempSurfaceModel = new ve.dm.Surface( 
selectionDocument ),
+                               tempFragment = 
tempSurfaceModel.getLinearFragment(
+                                       // TODO: Select all content using 
content offset methods
+                                       new ve.Range( 1, 
selectionDocument.getInternalList().getListNode().getOuterRange().start - 1 )
+                               );
+
+                       return tempFragment;
+               } );
+       } else {
+               fragmentPromise = $.Deferred().resolve( fragment ).promise();
        }
 
-       data = ve.extendObject( { dir: dir }, data, { fragment: fragment, 
$returnFocusTo: $noFocus } );
-       if ( windowType === 'toolbar' || windowType === 'inspector' ) {
-               data = ve.extendObject( data, { surface: surface } );
-               // Auto-close the current window if it is different to the one 
we are
-               // trying to open.
-               // TODO: Make auto-close a window manager setting
-               if ( currentWindow && currentWindow.constructor.static.name !== 
name ) {
-                       autoClosePromises.push( windowManager.closeWindow( 
currentWindow ) );
-               }
-       }
-
-       // If we're opening a dialog, close all inspectors first
-       if ( windowType === 'dialog' ) {
-               inspectorWindowManager = this.getWindowManager( 'inspector' );
-               currentInspector = inspectorWindowManager.getCurrentWindow();
-               if ( currentInspector ) {
-                       autoClosePromises.push( 
inspectorWindowManager.closeWindow( currentInspector ) );
-               }
-       }
-
-       $.when.apply( $, autoClosePromises ).always( function () {
-               windowManager.getWindow( name ).then( function ( win ) {
-                       var opening = windowManager.openWindow( win, data );
-
-                       if ( !win.constructor.static.activeSurface ) {
-                               surface.getView().deactivate();
+       fragmentPromise.then( function ( fragment ) {
+               data = ve.extendObject( { dir: dir }, data, { fragment: 
fragment, $returnFocusTo: $noFocus } );
+               if ( windowType === 'toolbar' || windowType === 'inspector' ) {
+                       data = ve.extendObject( data, { surface: surface } );
+                       // Auto-close the current window if it is different to 
the one we are
+                       // trying to open.
+                       // TODO: Make auto-close a window manager setting
+                       if ( currentWindow && 
currentWindow.constructor.static.name !== name ) {
+                               autoClosePromises.push( 
windowManager.closeWindow( currentWindow ) );
                        }
+               }
 
-                       opening.then( function ( closing ) {
-                               if ( sourceMode ) {
-                                       // HACK: previousSelection is assumed 
to be in the visible surface
-                                       win.previousSelection = null;
+               // If we're opening a dialog, close all inspectors first
+               if ( windowType === 'dialog' ) {
+                       inspectorWindowManager = this.getWindowManager( 
'inspector' );
+                       currentInspector = 
inspectorWindowManager.getCurrentWindow();
+                       if ( currentInspector ) {
+                               autoClosePromises.push( 
inspectorWindowManager.closeWindow( currentInspector ) );
+                       }
+               }
+
+               $.when.apply( $, autoClosePromises ).always( function () {
+                       windowManager.getWindow( name ).then( function ( win ) {
+                               var opening = windowManager.openWindow( win, 
data );
+
+                               if ( !win.constructor.static.activeSurface ) {
+                                       surface.getView().deactivate();
                                }
-                               closing.then( function ( closed ) {
-                                       if ( 
!win.constructor.static.activeSurface ) {
-                                               surface.getView().activate();
+
+                               opening.then( function ( closing ) {
+                                       if ( sourceMode ) {
+                                               // HACK: previousSelection is 
assumed to be in the visible surface
+                                               win.previousSelection = null;
                                        }
-                                       closed.then( function ( closedData ) {
-                                               // Sequence-triggered window 
closed without action, undo
-                                               if ( data.strippedSequence && 
!( closedData && closedData.action ) ) {
-                                                       
surface.getModel().undo();
+                                       closing.then( function ( closed ) {
+                                               if ( 
!win.constructor.static.activeSurface ) {
+                                                       
surface.getView().activate();
                                                }
-                                               if ( tempSurfaceModel && 
tempSurfaceModel.hasBeenModified() ) {
-                                                       
originalFragment.insertDocument( tempSurfaceModel.getDocument() );
-                                               }
-                                               surface.getView().emit( 
'position' );
+                                               closed.then( function ( 
closedData ) {
+                                                       // Sequence-triggered 
window closed without action, undo
+                                                       if ( 
data.strippedSequence && !( closedData && closedData.action ) ) {
+                                                               
surface.getModel().undo();
+                                                       }
+                                                       if ( sourceMode && 
fragment.getSurface().hasBeenModified() ) {
+                                                               
originalFragment.insertDocument( fragment.getDocument() );
+                                                       }
+                                                       surface.getView().emit( 
'position' );
+                                               } );
                                        } );
+                               } ).always( function () {
+                                       if ( action ) {
+                                               win.executeAction( action );
+                                       }
                                } );
-                       } ).always( function () {
-                               if ( action ) {
-                                       win.executeAction( action );
-                               }
                        } );
                } );
        } );

-- 
To view, visit https://gerrit.wikimedia.org/r/324777
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaa41c49e2f8e28af28a8a64cb6915f705fa35e8c
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <esand...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to