Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/151046
Change subject: [WIP] Use a single unicorn <img> instead of  in inline
slugs
......................................................................
[WIP] Use a single unicorn <img> instead of  in inline slugs
Seems to have the same effect for cursor behavior, and does not involve
text that we need the observer to ignore.
Changes:
* Replace FEFF with img in inlineSlugTemplate
* Remove ve-ce-branchNode class from inlineSlugTemplate to avoid
triggering slug-specific logic (HACK)
* Stop pawning inline slugs (stop all pawning; but nothing else used it)
* When text is inserted at a content offset where no TextNode exists yet,
don't tell DocumentSynchronizer to rebuild the entire CBN, but instead
tell it to just insert a TextNode
* Set ce.TextNode's $element to an empty collection to make
ce.BranchNode#onSplice's
DOM manipulation a no-op if only TextNodes are spliced in. Maybe we should
also respect the render lock in CBN onSplice
* Respect render lock in CBN setupSlugs
Alternative approach I tried and rejected:
* Change dm.Document#buildNodeTree to insert empty TextNodes at all
inline slug positions (before and after inline nodes, and at the
start and end of a CBN)
* Change DocumentSynchronizer to no longer prune TextNodes whose length
is reduced to zero
* This means TransactionProcessor can simply do a resize of the TextNode
that's already there. No splices occur, no CBN DOM manipulation to prevent
* This works in empty paragraphs but causes inline slugs to no longer appear
in other places where they're needed; fixing this would require reworking
the logic in setupSlugs
* Dragging an image to next to another image didn't cause empty dm.TextNodes
to be inserted around it: this is because DocumentSynchronizer would insert
the new node into the CBN rather than rebuilding the entire CBN
* This is probably fixable as well, but I preferred a CE solution to the problem
rather than polluting the DM with empty TextNodes.
TODO:
* Actually remove pawning code
* Actually remove / clean up slug-specific handling
* Redo render lock checking in a more carefully considered way
* Clean up check for pure text insertion case in TransactionProcessor
Change-Id: Id360d90d6cf6adad29b1f5f14bc27b7dc8109851
---
M modules/ve/ce/nodes/ve.ce.TextNode.js
M modules/ve/ce/ve.ce.BranchNode.js
M modules/ve/ce/ve.ce.ContentBranchNode.js
M modules/ve/ce/ve.ce.Surface.js
M modules/ve/dm/ve.dm.DocumentSynchronizer.js
M modules/ve/dm/ve.dm.TransactionProcessor.js
6 files changed, 41 insertions(+), 3 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/46/151046/1
diff --git a/modules/ve/ce/nodes/ve.ce.TextNode.js
b/modules/ve/ce/nodes/ve.ce.TextNode.js
index 589840e..d55d908 100644
--- a/modules/ve/ce/nodes/ve.ce.TextNode.js
+++ b/modules/ve/ce/nodes/ve.ce.TextNode.js
@@ -17,6 +17,8 @@
ve.ce.TextNode = function VeCeTextNode( model, config ) {
// Parent constructor
ve.ce.LeafNode.call( this, model, config );
+
+ this.$element = $( [] );
};
/* Inheritance */
diff --git a/modules/ve/ce/ve.ce.BranchNode.js
b/modules/ve/ce/ve.ce.BranchNode.js
index 66e04a6..949e3cc 100644
--- a/modules/ve/ce/ve.ce.BranchNode.js
+++ b/modules/ve/ce/ve.ce.BranchNode.js
@@ -61,8 +61,12 @@
* @property {jQuery}
*/
ve.ce.BranchNode.$inlineSlugTemplate = $( '<span>' )
- .addClass( 've-ce-branchNode-slug ve-ce-branchNode-inlineSlug' )
- .html( '' );
+ .addClass( /*'ve-ce-branchNode-slug*/ 've-ce-branchNode-inlineSlug' )
+ //.html( '' );
+ .append(
+ $( '<img>' )
+ .attr( 'src', ve.debug ? ve.ce.unicornImgDataUri :
ve.minImgDataUri )
+ );
/**
* Block slug template.
diff --git a/modules/ve/ce/ve.ce.ContentBranchNode.js
b/modules/ve/ce/ve.ce.ContentBranchNode.js
index 9b59bc5..6211105 100644
--- a/modules/ve/ce/ve.ce.ContentBranchNode.js
+++ b/modules/ve/ce/ve.ce.ContentBranchNode.js
@@ -113,6 +113,16 @@
this.renderContents();
};
+ve.ce.ContentBranchNode.prototype.setupSlugs = function () {
+ if (
+ this.root instanceof ve.ce.DocumentNode &&
+ this.root.getSurface().isRenderingLocked()
+ ) {
+ return;
+ }
+ ve.ce.BranchNode.prototype.setupSlugs.apply( this, arguments );
+};
+
/**
* Get an HTML rendering of the contents.
*
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index 8882e26..8dbc0ce 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -1977,7 +1977,7 @@
slug = this.documentView.getSlugAtOffset( selection.start );
// Always pawn in a slug
//if ( slug || this.needsPawn( selection, insertionAnnotations
) ) {
- if ( slug ) {
+ if ( slug && false ) {
placeholder = '♙';
if ( !insertionAnnotations.isEmpty() ) {
placeholder = [placeholder,
insertionAnnotations.getIndexes()];
diff --git a/modules/ve/dm/ve.dm.DocumentSynchronizer.js
b/modules/ve/dm/ve.dm.DocumentSynchronizer.js
index fc9c201..30d0d18 100644
--- a/modules/ve/dm/ve.dm.DocumentSynchronizer.js
+++ b/modules/ve/dm/ve.dm.DocumentSynchronizer.js
@@ -111,6 +111,12 @@
this.adjustment += action.adjustment;
};
+ve.dm.DocumentSynchronizer.synchronizers.insertTextNode = function ( action ) {
+ var textNode = new ve.dm.TextNode();
+ textNode.setLength( action.length );
+ action.parentNode.splice( action.index, 0, textNode );
+};
+
/**
* Synchronize a rebuild action.
*
@@ -214,6 +220,15 @@
} );
};
+ve.dm.DocumentSynchronizer.prototype.pushInsertTextNode = function (
parentNode, index, length ) {
+ this.actionQueue.push( {
+ type: 'insertTextNode',
+ parentNode: parentNode,
+ index: index,
+ length: length
+ } );
+};
+
/**
* Add a rebuild action to the queue.
*
diff --git a/modules/ve/dm/ve.dm.TransactionProcessor.js
b/modules/ve/dm/ve.dm.TransactionProcessor.js
index 281859f..c1fc148 100644
--- a/modules/ve/dm/ve.dm.TransactionProcessor.js
+++ b/modules/ve/dm/ve.dm.TransactionProcessor.js
@@ -363,6 +363,13 @@
// Text-only replacement
// Queue a resize for the text node
this.synchronizer.pushResize( node, insert.length -
remove.length );
+ } else if (
+ !removeHasStructure && !insertHasStructure &&
remove.length === 0 && insert.length > 0 &&
+ selection.length === 1 && node &&
node.canContainContent() &&
+ ( selection[0].indexInNode !== undefined ||
node.getLength() === 0 )
+ ) {
+ // Text-only addition where a text node didn't exist
before. Create one
+ this.synchronizer.pushInsertTextNode( node,
selection[0].indexInNode || 0, insert.length - remove.length );
} else {
// Replacement is not exclusively text
// Rebuild all covered nodes
--
To view, visit https://gerrit.wikimedia.org/r/151046
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id360d90d6cf6adad29b1f5f14bc27b7dc8109851
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan.katt...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
