jenkins-bot has submitted this change and it was merged.
Change subject: Make triple-click expand to CBNs, not BranchNodes
......................................................................
Make triple-click expand to CBNs, not BranchNodes
We used to expand to the nearest BranchNode, but really this
only makes sense for CBNs. Restricting ourselves to CBNs also
ensures that adjusting by (1, -1) is sane (for the DocumentNode
it isn't).
To deal with selections like |</p><p>|, collapse the selection
before expanding. This doesn't change the end result for selections
entirely contained within one CBN, but it helps us not go crazy
when the selection spans a CBN boundary.
Bug: T93461
Change-Id: Ib7338c6493963ae1f7f092739ac4ec90107a0ae1
---
M .jsduck/eg-iframe.html
M build/modules.json
M demos/ve/desktop.html
M demos/ve/mobile.html
M src/ce/ve.ce.Surface.js
M src/dm/nodes/ve.dm.HeadingNode.js
M src/dm/nodes/ve.dm.ParagraphNode.js
M src/dm/nodes/ve.dm.PreformattedNode.js
A src/dm/ve.dm.ContentBranchNode.js
M tests/index.html
10 files changed, 58 insertions(+), 7 deletions(-)
Approvals:
Divec: Looks good to me, approved
jenkins-bot: Verified
diff --git a/.jsduck/eg-iframe.html b/.jsduck/eg-iframe.html
index 34952b8..24ab260 100644
--- a/.jsduck/eg-iframe.html
+++ b/.jsduck/eg-iframe.html
@@ -157,6 +157,7 @@
<script src="../src/dm/ve.dm.ResizableNode.js"></script>
<script src="../src/dm/ve.dm.Node.js"></script>
<script src="../src/dm/ve.dm.BranchNode.js"></script>
+ <script src="../src/dm/ve.dm.ContentBranchNode.js"></script>
<script src="../src/dm/ve.dm.LeafNode.js"></script>
<script src="../src/dm/ve.dm.Annotation.js"></script>
<script src="../src/dm/ve.dm.InternalList.js"></script>
diff --git a/build/modules.json b/build/modules.json
index ecf3923..007b66c 100644
--- a/build/modules.json
+++ b/build/modules.json
@@ -185,6 +185,7 @@
"src/dm/ve.dm.ResizableNode.js",
"src/dm/ve.dm.Node.js",
"src/dm/ve.dm.BranchNode.js",
+ "src/dm/ve.dm.ContentBranchNode.js",
"src/dm/ve.dm.LeafNode.js",
"src/dm/ve.dm.Annotation.js",
"src/dm/ve.dm.InternalList.js",
diff --git a/demos/ve/desktop.html b/demos/ve/desktop.html
index 9d51cf1..c562a15 100644
--- a/demos/ve/desktop.html
+++ b/demos/ve/desktop.html
@@ -174,6 +174,7 @@
<script src="../../src/dm/ve.dm.ResizableNode.js"></script>
<script src="../../src/dm/ve.dm.Node.js"></script>
<script src="../../src/dm/ve.dm.BranchNode.js"></script>
+ <script src="../../src/dm/ve.dm.ContentBranchNode.js"></script>
<script src="../../src/dm/ve.dm.LeafNode.js"></script>
<script src="../../src/dm/ve.dm.Annotation.js"></script>
<script src="../../src/dm/ve.dm.InternalList.js"></script>
diff --git a/demos/ve/mobile.html b/demos/ve/mobile.html
index d0ece5d..464e723 100644
--- a/demos/ve/mobile.html
+++ b/demos/ve/mobile.html
@@ -175,6 +175,7 @@
<script src="../../src/dm/ve.dm.ResizableNode.js"></script>
<script src="../../src/dm/ve.dm.Node.js"></script>
<script src="../../src/dm/ve.dm.BranchNode.js"></script>
+ <script src="../../src/dm/ve.dm.ContentBranchNode.js"></script>
<script src="../../src/dm/ve.dm.LeafNode.js"></script>
<script src="../../src/dm/ve.dm.Annotation.js"></script>
<script src="../../src/dm/ve.dm.InternalList.js"></script>
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index d948ec7..aa39f2c 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -783,6 +783,7 @@
* @param {jQuery.Event} e Mouse down event
*/
ve.ce.Surface.prototype.onDocumentMouseDown = function ( e ) {
+ var newFragment;
if ( e.which !== 1 ) {
return;
}
@@ -804,7 +805,23 @@
// Browser default behaviour for triple click won't behave as
we want
e.preventDefault();
- this.getModel().getFragment().expandLinearSelection( 'closest',
ve.dm.BranchNode ).adjustLinearSelection( 1, -1 ).select();
+ newFragment = this.getModel().getFragment()
+ // After double-clicking in an inline slug, we'll get a
selection like
+ // <p><span><img />|</span></p><p>|Foo</p>. This
selection spans a CBN boundary,
+ // so we can't expand to the nearest CBN. To handle
this case and other possible
+ // cases where the selection spans a CBN boundary,
collapse the selection before
+ // expanding it. If the selection is entirely within
the same CBN as it should be,
+ // this won't change the result.
+ .collapseToStart()
+ // Cover the CBN we're in
+ .expandLinearSelection( 'closest',
ve.dm.ContentBranchNode )
+ // ...but that covered the entire CBN, we only want the
contents
+ .adjustLinearSelection( 1, -1 );
+ // If something weird happened (e.g. no CBN found), newFragment
will be null.
+ // Don't select it in that case, because that'll blur the
surface.
+ if ( !newFragment.isNull() ) {
+ newFragment.select();
+ }
}
};
diff --git a/src/dm/nodes/ve.dm.HeadingNode.js
b/src/dm/nodes/ve.dm.HeadingNode.js
index 7e55a64..4112a58 100644
--- a/src/dm/nodes/ve.dm.HeadingNode.js
+++ b/src/dm/nodes/ve.dm.HeadingNode.js
@@ -8,7 +8,7 @@
* DataModel heading node.
*
* @class
- * @extends ve.dm.BranchNode
+ * @extends ve.dm.ContentBranchNode
*
* @constructor
* @param {Object} [element] Reference to element in linear model
@@ -21,7 +21,7 @@
/* Inheritance */
-OO.inheritClass( ve.dm.HeadingNode, ve.dm.BranchNode );
+OO.inheritClass( ve.dm.HeadingNode, ve.dm.ContentBranchNode );
/* Static Properties */
diff --git a/src/dm/nodes/ve.dm.ParagraphNode.js
b/src/dm/nodes/ve.dm.ParagraphNode.js
index 4ccd94a..359c4fd 100644
--- a/src/dm/nodes/ve.dm.ParagraphNode.js
+++ b/src/dm/nodes/ve.dm.ParagraphNode.js
@@ -8,7 +8,7 @@
* DataModel paragraph node.
*
* @class
- * @extends ve.dm.BranchNode
+ * @extends ve.dm.ContentBranchNode
*
* @constructor
* @param {Object} [element] Reference to element in linear model
@@ -21,7 +21,7 @@
/* Inheritance */
-OO.inheritClass( ve.dm.ParagraphNode, ve.dm.BranchNode );
+OO.inheritClass( ve.dm.ParagraphNode, ve.dm.ContentBranchNode );
/* Static Properties */
diff --git a/src/dm/nodes/ve.dm.PreformattedNode.js
b/src/dm/nodes/ve.dm.PreformattedNode.js
index 863a291..1823390 100644
--- a/src/dm/nodes/ve.dm.PreformattedNode.js
+++ b/src/dm/nodes/ve.dm.PreformattedNode.js
@@ -8,7 +8,7 @@
* DataModel preformatted node.
*
* @class
- * @extends ve.dm.BranchNode
+ * @extends ve.dm.ContentBranchNode
*
* @constructor
* @param {Object} [element] Reference to element in linear model
@@ -21,7 +21,7 @@
/* Inheritance */
-OO.inheritClass( ve.dm.PreformattedNode, ve.dm.BranchNode );
+OO.inheritClass( ve.dm.PreformattedNode, ve.dm.ContentBranchNode );
/* Static Properties */
diff --git a/src/dm/ve.dm.ContentBranchNode.js
b/src/dm/ve.dm.ContentBranchNode.js
new file mode 100644
index 0000000..c75a598
--- /dev/null
+++ b/src/dm/ve.dm.ContentBranchNode.js
@@ -0,0 +1,29 @@
+/*!
+ * VisualEditor DataModel ContentBranchNode class.
+ *
+ * @copyright 2011-2015 VisualEditor Team and others; see
http://ve.mit-license.org
+ */
+
+/**
+ * DataModel content branch node.
+ *
+ * @class
+ * @abstract
+ * @extends ve.dm.BranchNode
+ *
+ * @constructor
+ * @param {Object} [element] Reference to element in linear model
+ * @param {ve.dm.Node[]} [children]
+ */
+ve.dm.ContentBranchNode = function VeDmContentBranchNode() {
+ // Parent constructor
+ ve.dm.ContentBranchNode.super.apply( this, arguments );
+};
+
+/* Inheritance */
+
+OO.inheritClass( ve.dm.ContentBranchNode, ve.dm.BranchNode );
+
+/* Static Properties */
+
+ve.dm.ContentBranchNode.static.canContainContent = true;
diff --git a/tests/index.html b/tests/index.html
index 1c2ac38..b4f846c 100644
--- a/tests/index.html
+++ b/tests/index.html
@@ -109,6 +109,7 @@
<script src="../src/dm/ve.dm.ResizableNode.js"></script>
<script src="../src/dm/ve.dm.Node.js"></script>
<script src="../src/dm/ve.dm.BranchNode.js"></script>
+ <script src="../src/dm/ve.dm.ContentBranchNode.js"></script>
<script src="../src/dm/ve.dm.LeafNode.js"></script>
<script src="../src/dm/ve.dm.Annotation.js"></script>
<script src="../src/dm/ve.dm.InternalList.js"></script>
--
To view, visit https://gerrit.wikimedia.org/r/199565
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib7338c6493963ae1f7f092739ac4ec90107a0ae1
Gerrit-PatchSet: 2
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Divec <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
