jenkins-bot has submitted this change and it was merged.
Change subject: Add ve.ce.View as a common base class for ce.Node and
ce.Annotation
......................................................................
Add ve.ce.View as a common base class for ce.Node and ce.Annotation
Just like ve.dm.Model is a common base class for dm.Node, dm.Annotation
and dm.MetaItem.
ce.View abstracts the this.model and this.$ behavior, including liveness,
whitelisted HTML attribute rendering and adding a back reference in
.data(). The back reference has been renamed from .data( 'node' ) to
the more generic .data( 'view' ).
At this point this means ce.Annotation is just a shell around ce.View
(except where it defaults to a span rather than a div), but that could
change in the future.
Change-Id: I0eef5b80718e0b0fcd3f8bba096b452f0bb680d0
---
M .docs/categories.json
M VisualEditor.php
M demos/ve/index.php
M modules/ve/ce/ve.ce.Annotation.js
M modules/ve/ce/ve.ce.BranchNode.js
M modules/ve/ce/ve.ce.Document.js
M modules/ve/ce/ve.ce.Node.js
M modules/ve/ce/ve.ce.SurfaceObserver.js
A modules/ve/ce/ve.ce.View.js
M modules/ve/ce/ve.ce.js
M modules/ve/dm/ve.dm.MetaItem.js
M modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
M modules/ve/test/index.php
13 files changed, 143 insertions(+), 174 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/.docs/categories.json b/.docs/categories.json
index 33b7ea7..b37471d 100644
--- a/.docs/categories.json
+++ b/.docs/categories.json
@@ -15,6 +15,7 @@
"name": "General",
"classes": [
"ve.ce",
+ "ve.ce.View",
"ve.ce.AnnotationFactory",
"ve.ce.NodeFactory",
"ve.ce.Surface",
diff --git a/VisualEditor.php b/VisualEditor.php
index 752e7ee..5b3e5f0 100644
--- a/VisualEditor.php
+++ b/VisualEditor.php
@@ -290,6 +290,7 @@
've/ce/ve.ce.AnnotationFactory.js',
've/ce/ve.ce.NodeFactory.js',
've/ce/ve.ce.Document.js',
+ 've/ce/ve.ce.View.js',
've/ce/ve.ce.Annotation.js',
've/ce/ve.ce.Node.js',
've/ce/ve.ce.BranchNode.js',
diff --git a/demos/ve/index.php b/demos/ve/index.php
index d48b9e0..eb342d3 100644
--- a/demos/ve/index.php
+++ b/demos/ve/index.php
@@ -172,6 +172,7 @@
<script
src="../../modules/ve/ce/ve.ce.AnnotationFactory.js"></script>
<script src="../../modules/ve/ce/ve.ce.NodeFactory.js"></script>
<script src="../../modules/ve/ce/ve.ce.Document.js"></script>
+ <script src="../../modules/ve/ce/ve.ce.View.js"></script>
<script src="../../modules/ve/ce/ve.ce.Annotation.js"></script>
<script src="../../modules/ve/ce/ve.ce.Node.js"></script>
<script src="../../modules/ve/ce/ve.ce.BranchNode.js"></script>
@@ -424,8 +425,8 @@
validateButton.on( 'click', function () {
var failed = false;
$('.ve-ce-branchNode').each( function ( index,
element ) {
- var $element = $( element ),
- view = $element.data( 'node' );
+ var $element = $( element ),
+ view = $element.data( 'view' );
if ( view.canContainContent() ) {
var nodeRange =
view.model.getRange();
var textModel =
ve.instances[0].view.model.getDocument().getText( nodeRange );
diff --git a/modules/ve/ce/ve.ce.Annotation.js
b/modules/ve/ce/ve.ce.Annotation.js
index 66cc053..e172f19 100644
--- a/modules/ve/ce/ve.ce.Annotation.js
+++ b/modules/ve/ce/ve.ce.Annotation.js
@@ -13,93 +13,18 @@
*
* Subclasses of ve.dm.Annotation should have a corresponding subclass here
that controls rendering.
*
- * @class
+ * @abstract
+ * @extends ve.ce.View
+ *
* @constructor
* @param {ve.dm.Annotation} model Model to observe
* @param {jQuery} [$element] Element to use as a container
*/
ve.ce.Annotation = function VeCeAnnotation( model, $element ) {
- // Properties
- this.model = model;
- this.$ = $element || $( '<span>' );
- this.parent = null;
- this.live = false;
-
- // Initialization
- this.$.data( 'annotation', this );
- ve.setDomAttributes(
- this.$[0],
- this.model.getAttributes( 'html/0/' ),
- this.constructor.static.domAttributeWhitelist
- );
+ // Parent constructor
+ ve.ce.View.call( this, model, $element || $( '<span>' ) );
};
-/* Events */
+/* Inheritance */
-/**
- * @event live
- */
-
-/* Static Members */
-
-// TODO create a single base class for ce.Node and ce.Annotation
-
-ve.ce.Annotation.static = {};
-
-/**
- * Allowed attributes for DOM elements.
- *
- * This list includes attributes that are generally safe to include in HTML
loaded from a
- * foreign source and displaying it inside the browser. It doesn't include any
event attributes,
- * for instance, which would allow arbitrary JavaScript execution. This alone
is not enough to
- * make HTML safe to display, but it helps.
- *
- * TODO: Rather than use a single global list, set these on a per-annotation
basis to something that makes
- * sense for that annotation in particular.
- *
- * @static
- * @property static.domAttributeWhitelist
- * @inheritable
- */
-ve.ce.Annotation.static.domAttributeWhitelist = [
- 'abbr', 'about', 'align', 'alt', 'axis', 'bgcolor', 'border',
'cellpadding', 'cellspacing',
- 'char', 'charoff', 'cite', 'class', 'clear', 'color', 'colspan',
'datatype', 'datetime',
- 'dir', 'face', 'frame', 'headers', 'height', 'href', 'id', 'itemid',
'itemprop', 'itemref',
- 'itemscope', 'itemtype', 'lang', 'noshade', 'nowrap', 'property',
'rbspan', 'rel',
- 'resource', 'rev', 'rowspan', 'rules', 'scope', 'size', 'span', 'src',
'start', 'style',
- 'summary', 'title', 'type', 'typeof', 'valign', 'value', 'width'
-];
-
-/* Methods */
-
-/**
- * Get the model this CE annotation observes.
- *
- * @method
- * @returns {ve.ce.Annotation} Model
- */
-ve.ce.Annotation.prototype.getModel = function () {
- return this.model;
-};
-
-/**
- * Check if the annotation is attached to the live DOM.
- *
- * @method
- * @returns {boolean} Annotation is attached to the live DOM
- */
-ve.ce.Annotation.prototype.isLive = function () {
- return this.live;
-};
-
-/**
- * Set live state.
- *
- * @method
- * @param {boolean} live The annotation has been attached to the live DOM (use
false on detach)
- * @emits live
- */
-ve.ce.Annotation.prototype.setLive = function ( live ) {
- this.live = live;
- this.emit( 'live' );
-};
+ve.inheritClass( ve.ce.Annotation, ve.ce.View );
diff --git a/modules/ve/ce/ve.ce.BranchNode.js
b/modules/ve/ce/ve.ce.BranchNode.js
index a4f02f5..203771c 100644
--- a/modules/ve/ce/ve.ce.BranchNode.js
+++ b/modules/ve/ce/ve.ce.BranchNode.js
@@ -129,7 +129,7 @@
*
* This method uses {getDomWrapperType} to determine the proper element type
to use.
*
- * WARNING: The contents, .data( 'node' ) and any classes the wrapper already
has will be moved to
+ * WARNING: The contents, .data( 'view' ) and any classes the wrapper already
has will be moved to
* the new wrapper, but other attributes and any other information added using
$.data() will be
* lost upon updating the wrapper. To retain information added to the wrapper,
subscribe to the
* 'rewrap' event and copy information from the {$old} wrapper the {$new}
wrapper.
@@ -146,8 +146,8 @@
$element = $( document.createElement( type ) );
// Copy classes
$element.attr( 'class', this.$.attr( 'class' ) );
- // Copy .data( 'node' )
- $element.data( 'node', this.$.data( 'node' ) );
+ // Copy .data( 'view' )
+ $element.data( 'view', this.$.data( 'view' ) );
// Move contents
$element.append( this.$.contents() );
// Emit an event that can be handled to copy other things over
if needed
diff --git a/modules/ve/ce/ve.ce.Document.js b/modules/ve/ce/ve.ce.Document.js
index b6610b6..78ea471 100644
--- a/modules/ve/ce/ve.ce.Document.js
+++ b/modules/ve/ce/ve.ce.Document.js
@@ -216,7 +216,7 @@
};
}
} else if ( $item.is( '.ve-ce-branchNode,
.ve-ce-leafNode' ) ) {
- length = $item.data( 'node'
).model.getOuterLength();
+ length = $item.data( 'view'
).model.getOuterLength();
if ( offset >= startOffset && offset <
startOffset + length ) {
stack.push( [$item.contents(), 0] );
current[1]++;
diff --git a/modules/ve/ce/ve.ce.Node.js b/modules/ve/ce/ve.ce.Node.js
index 55fd1fb..f099adb 100644
--- a/modules/ve/ce/ve.ce.Node.js
+++ b/modules/ve/ce/ve.ce.Node.js
@@ -9,7 +9,8 @@
* Generic ContentEditable node.
*
* @abstract
- * @extends ve.Node
+ * @extends ve.ce.View
+ * @mixins ve.Node
*
* @constructor
* @param {ve.dm.Node} model Model to observe
@@ -17,35 +18,22 @@
*/
ve.ce.Node = function VeCeNode( model, $element ) {
// Parent constructor
+ ve.ce.View.call( this, model, $element );
+ // Mixin constructor
ve.Node.call( this );
// Properties
- this.model = model;
- this.$ = $element || $( '<div>' );
this.parent = null;
- this.live = false;
// Events
this.model.on( 'attributeChange', ve.bind( this.onAttributeChange, this
) );
-
- // Initialization
- this.$.data( 'node', this );
- ve.setDomAttributes(
- this.$[0],
- this.model.getAttributes( 'html/0/' ),
- this.constructor.static.domAttributeWhitelist
- );
};
/* Inheritance */
-ve.inheritClass( ve.ce.Node, ve.Node );
+ve.inheritClass( ve.ce.Node, ve.ce.View );
-/* Events */
-
-/**
- * @event live
- */
+ve.mixinClass( ve.ce.Node, ve.Node );
/* Static Members */
@@ -60,30 +48,6 @@
* @inheritable
*/
ve.ce.Node.static.canBeSplit = false;
-
-/**
- * Allowed attributes for DOM elements.
- *
- * This list includes attributes that are generally safe to include in HTML
loaded from a
- * foreign source and displaying it inside the browser. It doesn't include any
event attributes,
- * for instance, which would allow arbitrary JavaScript execution. This alone
is not enough to
- * make HTML safe to display, but it helps.
- *
- * TODO: Rather than use a single global list, set these on a per-node basis
to something that makes
- * sense for that node in particular.
- *
- * @static
- * @property static.domAttributeWhitelist
- * @inheritable
- */
-ve.ce.Node.static.domAttributeWhitelist = [
- 'abbr', 'about', 'align', 'alt', 'axis', 'bgcolor', 'border',
'cellpadding', 'cellspacing',
- 'char', 'charoff', 'cite', 'class', 'clear', 'color', 'colspan',
'datatype', 'datetime',
- 'dir', 'face', 'frame', 'headers', 'height', 'href', 'id', 'itemid',
'itemprop', 'itemref',
- 'itemscope', 'itemtype', 'lang', 'noshade', 'nowrap', 'property',
'rbspan', 'rel',
- 'resource', 'rev', 'rowspan', 'rules', 'scope', 'size', 'span', 'src',
'start', 'style',
- 'summary', 'title', 'type', 'typeof', 'valign', 'value', 'width'
-];
/**
* Template for shield elements.
@@ -270,16 +234,6 @@
};
/**
- * Get the model the node observes.
- *
- * @method
- * @returns {ve.dm.Node} Model the node observes
- */
-ve.ce.Node.prototype.getModel = function () {
- return this.model;
-};
-
-/**
* Get the closest splittable node upstream.
*
* @method
@@ -298,26 +252,4 @@
} );
return splitableNode;
-};
-
-/**
- * Check if the node is attached to the live DOM.
- *
- * @method
- * @returns {boolean} Node is attached to the live DOM
- */
-ve.ce.Node.prototype.isLive = function () {
- return this.live;
-};
-
-/**
- * Set live state.
- *
- * @method
- * @param {boolean} live The node has been attached to the live DOM (use false
on detach)
- * @emits live
- */
-ve.ce.Node.prototype.setLive = function ( live ) {
- this.live = live;
- this.emit( 'live' );
};
diff --git a/modules/ve/ce/ve.ce.SurfaceObserver.js
b/modules/ve/ce/ve.ce.SurfaceObserver.js
index 79d9d31..53c4b61 100644
--- a/modules/ve/ce/ve.ce.SurfaceObserver.js
+++ b/modules/ve/ce/ve.ce.SurfaceObserver.js
@@ -157,7 +157,7 @@
this.rangyRange = rangyRange;
$branch = $( rangyRange.anchorNode ).closest(
'.ve-ce-branchNode' );
if ( $branch.length ) {
- node = $branch.data( 'node' );
+ node = $branch.data( 'view' );
if ( node.canHaveChildrenNotContent() ) {
node = null;
} else {
diff --git a/modules/ve/ce/ve.ce.View.js b/modules/ve/ce/ve.ce.View.js
new file mode 100644
index 0000000..cb9ae7e
--- /dev/null
+++ b/modules/ve/ce/ve.ce.View.js
@@ -0,0 +1,106 @@
+/*!
+ * VisualEditor ContentEditable View class.
+ *
+ * @copyright 2011-2013 VisualEditor Team and others; see AUTHORS.txt
+ * @license The MIT License (MIT); see LICENSE.txt
+ */
+
+/**
+ * Generic base class for CE views.
+ *
+ * @abstract
+ * @mixins ve.EventEmitter
+ *
+ * @constructor
+ * @param {ve.dm.Model} model Model to observe
+ * @param {jQuery} [$element] Element to use as a container
+ */
+ve.ce.View = function VeCeView( model, $element ) {
+ // Parent constructor
+ ve.EventEmitter.call( this );
+
+ // Properties
+ this.model = model;
+ this.$ = $element || $( '<div>' );
+ this.live = false;
+
+ // Initialization
+ this.$.data( 'view', this );
+ ve.setDomAttributes(
+ this.$[0],
+ this.model.getAttributes( 'html/0/' ),
+ this.constructor.static.domAttributeWhitelist
+ );
+};
+
+/* Inheritance */
+
+ve.mixinClass( ve.ce.View, ve.EventEmitter );
+
+/* Events */
+
+/**
+ * @event live
+ */
+
+/* Static Members */
+
+ve.ce.View.static = {};
+
+/**
+ * Allowed attributes for DOM elements.
+ *
+ * This list includes attributes that are generally safe to include in HTML
loaded from a
+ * foreign source and displaying it inside the browser. It doesn't include any
event attributes,
+ * for instance, which would allow arbitrary JavaScript execution. This alone
is not enough to
+ * make HTML safe to display, but it helps.
+ *
+ * TODO: Rather than use a single global list, set these on a per-view basis
to something that makes
+ * sense for that view in particular.
+ *
+ * @static
+ * @property static.domAttributeWhitelist
+ * @inheritable
+ */
+ve.ce.View.static.domAttributeWhitelist = [
+ 'abbr', 'about', 'align', 'alt', 'axis', 'bgcolor', 'border',
'cellpadding', 'cellspacing',
+ 'char', 'charoff', 'cite', 'class', 'clear', 'color', 'colspan',
'datatype', 'datetime',
+ 'dir', 'face', 'frame', 'headers', 'height', 'href', 'id', 'itemid',
'itemprop', 'itemref',
+ 'itemscope', 'itemtype', 'lang', 'noshade', 'nowrap', 'property',
'rbspan', 'rel',
+ 'resource', 'rev', 'rowspan', 'rules', 'scope', 'size', 'span', 'src',
'start', 'style',
+ 'summary', 'title', 'type', 'typeof', 'valign', 'value', 'width'
+];
+
+/* Methods */
+
+/**
+ * Get the model the view observes.
+ *
+ * @method
+ * @returns {ve.dm.Node} Model the view observes
+ */
+ve.ce.View.prototype.getModel = function () {
+ return this.model;
+};
+
+/**
+ * Check if the view is attached to the live DOM.
+ *
+ * @method
+ * @returns {boolean} View is attached to the live DOM
+ */
+ve.ce.View.prototype.isLive = function () {
+ return this.live;
+};
+
+/**
+ * Set live state.
+ *
+ * @method
+ * @param {boolean} live The view has been attached to the live DOM (use false
on detach)
+ * @emits live
+ */
+ve.ce.View.prototype.setLive = function ( live ) {
+ this.live = live;
+ this.emit( 'live' );
+};
diff --git a/modules/ve/ce/ve.ce.js b/modules/ve/ce/ve.ce.js
index fb6b29a..1c7511f 100644
--- a/modules/ve/ce/ve.ce.js
+++ b/modules/ve/ce/ve.ce.js
@@ -55,7 +55,7 @@
} else if ( $element.hasClass( 've-ce-leafNode' ) ) {
// For leaf nodes, don't return the content,
but return
// the right amount of characters so the
offsets match up
- numChars = $element.data( 'node'
).getOuterLength();
+ numChars = $element.data( 'view'
).getOuterLength();
return new Array( numChars + 1 ).join( '\u2603'
);
} else {
// Traverse its children
@@ -133,7 +133,7 @@
$node = $( domNode ).closest(
'.ve-ce-branchNode, .ve-ce-leafNode'
);
- nodeModel = $node.data( 'node' ).getModel();
+ nodeModel = $node.data( 'view' ).getModel();
// IE sometimes puts the cursor in a text node inside ce="false". BAD!
if ( $node[0].contentEditable === 'false' ) {
@@ -171,7 +171,7 @@
} else if ( $item.hasClass( 've-ce-leafNode' ) ) {
offset += 2;
} else if ( $item.hasClass( 've-ce-branchNode' ) ) {
- offset += $item.data( 'node' ).getOuterLength();
+ offset += $item.data( 'view' ).getOuterLength();
} else {
stack.push( [$item.contents(), 0 ] );
current[1]++;
@@ -200,25 +200,25 @@
if ( $domNode.hasClass( 've-ce-slug' ) ) {
if ( $domNode.prev().length ) {
- nodeModel = $domNode.prev().data( 'node' ).getModel();
+ nodeModel = $domNode.prev().data( 'view' ).getModel();
return nodeModel.getOffset() +
nodeModel.getOuterLength();
}
if ( $domNode.next().length ) {
- nodeModel = $domNode.next().data( 'node' ).getModel();
+ nodeModel = $domNode.next().data( 'view' ).getModel();
return nodeModel.getOffset();
}
}
// IE sometimes puts the cursor in a text node inside ce="false". BAD!
if ( domNode.contentEditable === 'false' ) {
- nodeModel = $domNode.data( 'node' ).getModel();
+ nodeModel = $domNode.data( 'view' ).getModel();
return nodeModel.getOffset() + nodeModel.getOuterLength();
}
if ( domOffset === 0 ) {
- node = $domNode.data( 'node' );
+ node = $domNode.data( 'view' );
if ( node ) {
- nodeModel = $domNode.data( 'node' ).getModel();
+ nodeModel = $domNode.data( 'view' ).getModel();
if ( addOuterLength === true ) {
return nodeModel.getOffset() +
nodeModel.getOuterLength();
} else {
@@ -249,10 +249,10 @@
ve.ce.getOffsetOfSlug = function ( $node ) {
var model;
if ( $node.index() === 0 ) {
- model = $node.parent().data( 'node' ).getModel();
+ model = $node.parent().data( 'view' ).getModel();
return model.getOffset() + ( model.isWrapped() ? 1 : 0 );
} else if ( $node.prev().length ) {
- model = $node.prev().data( 'node' ).getModel();
+ model = $node.prev().data( 'view' ).getModel();
return model.getOffset() + model.getOuterLength();
} else {
throw new Error( 'Incorrect slug location' );
diff --git a/modules/ve/dm/ve.dm.MetaItem.js b/modules/ve/dm/ve.dm.MetaItem.js
index e4e2595..738c312 100644
--- a/modules/ve/dm/ve.dm.MetaItem.js
+++ b/modules/ve/dm/ve.dm.MetaItem.js
@@ -11,6 +11,8 @@
* @class
* @abstract
* @extends ve.dm.Model
+ * @mixins ve.EventEmitter
+ *
* @constructor
* @param {Object} element Reference to element in meta-linmod
*/
diff --git a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
index 99a0df9..21dc153 100644
--- a/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
+++ b/modules/ve/init/mw/targets/ve.init.mw.ViewPageTarget.js
@@ -1541,7 +1541,7 @@
surfaceView = this.surface.getView(),
surfaceModel = surfaceView.getModel();
this.$document.find( 'h1, h2, h3, h4, h5, h6' ).eq(
this.section - 1 ).each( function () {
- var headingNode = $(this).data( 'node' );
+ var headingNode = $(this).data( 'view' );
if ( headingNode ) {
offset =
surfaceModel.getDocument().data.getNearestContentOffset(
headingNode.getModel().getOffset()
diff --git a/modules/ve/test/index.php b/modules/ve/test/index.php
index cbdf7a5..1ec8fe6 100644
--- a/modules/ve/test/index.php
+++ b/modules/ve/test/index.php
@@ -116,6 +116,7 @@
<script src="../../ve/ce/ve.ce.AnnotationFactory.js"></script>
<script src="../../ve/ce/ve.ce.NodeFactory.js"></script>
<script src="../../ve/ce/ve.ce.Document.js"></script>
+ <script src="../../ve/ce/ve.ce.View.js"></script>
<script src="../../ve/ce/ve.ce.Annotation.js"></script>
<script src="../../ve/ce/ve.ce.Node.js"></script>
<script src="../../ve/ce/ve.ce.BranchNode.js"></script>
--
To view, visit https://gerrit.wikimedia.org/r/57242
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0eef5b80718e0b0fcd3f8bba096b452f0bb680d0
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
