jenkins-bot has submitted this change and it was merged.
Change subject: Continue links
......................................................................
Continue links
This isn't as simple as just dropping applyToAppendedContent = false
on LinkAnnotation, because browsers differ in their continuation
behavior. Firefox continues links, but Chrome doesn't.
To work around this, add a property indicating that the annotation
needs its continuation behavior to be forced.
Rename areAnnotationsCorrect() to needsPawn() accordingly.
Bug: 49931
Change-Id: Id6424af89c92bba2be87736e8a937e0f2067c007
---
M modules/ve/ce/annotations/ve.ce.LinkAnnotation.js
M modules/ve/ce/ve.ce.Annotation.js
M modules/ve/ce/ve.ce.AnnotationFactory.js
M modules/ve/ce/ve.ce.Surface.js
M modules/ve/dm/annotations/ve.dm.LinkAnnotation.js
5 files changed, 65 insertions(+), 17 deletions(-)
Approvals:
Trevor Parscal: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/ce/annotations/ve.ce.LinkAnnotation.js
b/modules/ve/ce/annotations/ve.ce.LinkAnnotation.js
index 669ab07..8651617 100644
--- a/modules/ve/ce/annotations/ve.ce.LinkAnnotation.js
+++ b/modules/ve/ce/annotations/ve.ce.LinkAnnotation.js
@@ -33,6 +33,8 @@
ve.ce.LinkAnnotation.static.tagName = 'a';
+ve.ce.LinkAnnotation.static.forceContinuation = true;
+
/* Registration */
ve.ce.annotationFactory.register( ve.ce.LinkAnnotation );
diff --git a/modules/ve/ce/ve.ce.Annotation.js
b/modules/ve/ce/ve.ce.Annotation.js
index 10f007b..28b3780 100644
--- a/modules/ve/ce/ve.ce.Annotation.js
+++ b/modules/ve/ce/ve.ce.Annotation.js
@@ -32,3 +32,15 @@
/* Static Properties */
ve.ce.Annotation.static.tagName = 'span';
+
+/**
+ * Whether this annotation's continuation (or lack thereof) needs to be forced.
+ *
+ * This should be set to true only for annotations that aren't continued by
browsers but are in DM,
+ * or the other way around, or those where behavior is inconsistent between
browsers.
+ *
+ * @property static.forceContinuation
+ * @static
+ * @inheritable
+ */
+ve.ce.Annotation.static.forceContinuation = false;
diff --git a/modules/ve/ce/ve.ce.AnnotationFactory.js
b/modules/ve/ce/ve.ce.AnnotationFactory.js
index 350166a..1e6e13c 100644
--- a/modules/ve/ce/ve.ce.AnnotationFactory.js
+++ b/modules/ve/ce/ve.ce.AnnotationFactory.js
@@ -21,6 +21,20 @@
ve.inheritClass( ve.ce.AnnotationFactory, ve.NamedClassFactory );
+/* Methods */
+
+/**
+ * Check if an annotation needs to force continuation
+ * @param {string} type Annotation type
+ * @returns {boolean} Whether the annotation needs to force continuation
+ */
+ve.ce.AnnotationFactory.prototype.isAnnotationContinuationForced = function (
type ) {
+ if ( type in this.registry ) {
+ return this.registry[type].static.forceContinuation;
+ }
+ return false;
+};
+
/* Initialization */
// TODO: Move instantiation to a different file
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index 7b78e0c..e308f1c 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -1071,8 +1071,8 @@
if ( selection.isCollapsed() ) {
slug = this.documentView.getSlugAtOffset( selection.start );
- // Is this a slug or are the annotations incorrect?
- if ( slug || !this.areAnnotationsCorrect( selection,
insertionAnnotations ) ) {
+ // Always pawn in a slug
+ if ( slug || this.needsPawn( selection, insertionAnnotations )
) {
placeholder = '♙';
if ( !insertionAnnotations.isEmpty() ) {
placeholder = [placeholder,
insertionAnnotations.getIndexes()];
@@ -1442,33 +1442,55 @@
};
/**
- * Checks if related annotationSet matches insertionAnnotations.
+ * Checks if we need to pawn for insertionAnnotations based on the related
annotationSet.
*
* "Related" is typically to the left, unless at the beginning of a node.
*
+ * We choose to pawn if the related annotationSet doesn't match
insertionAnnotations, or if
+ * we are at the edge of an annotation that requires pawning (i.e. an
annotation requiring pawning
+ * is present on the left but not on the right, or vice versa).
+ *
* @method
* @param {ve.Range} selection
- * @returns {ve.dm.AnnotationSet} insertionAnnotations
+ * @param {ve.dm.AnnotationSet} insertionAnnotations
+ * @returns {boolean} Whether we need to pawn
*/
-ve.ce.Surface.prototype.areAnnotationsCorrect = function ( selection,
insertionAnnotations ) {
- var documentModel = this.model.documentModel;
+ve.ce.Surface.prototype.needsPawn = function ( selection, insertionAnnotations
) {
+ var leftAnnotations, rightAnnotations, documentModel =
this.model.documentModel;
+
+ function isForced( annotation ) {
+ return ve.ce.annotationFactory.isAnnotationContinuationForced(
annotation.constructor.static.name );
+ }
+
+ if ( selection.start > 0 ) {
+ leftAnnotations = documentModel.data.getAnnotationsFromOffset(
selection.start - 1 );
+ }
+ if ( selection.start < documentModel.data.getLength() ) {
+ rightAnnotations = documentModel.data.getAnnotationsFromOffset(
selection.start + 1 );
+ }
// Take annotations from the left
- if (
- selection.start > 0 &&
- !documentModel.data.getAnnotationsFromOffset( selection.start -
1 ).compareTo( insertionAnnotations )
- ) {
- return false;
+ // TODO reorganize the logic in this function
+ if ( leftAnnotations && !leftAnnotations.compareTo(
insertionAnnotations ) ) {
+ return true;
}
// At the beginning of a node, take from the right
if (
rangy.getSelection( this.$document[0] ).anchorOffset === 0 &&
- selection.start < this.model.getDocument().data.getLength() &&
- !documentModel.data.getAnnotationsFromOffset( selection.start +
1 ).compareTo( insertionAnnotations )
+ rightAnnotations &&
+ !rightAnnotations.compareTo( insertionAnnotations )
) {
- return false;
+ return true;
}
- return true;
+
+ if (
+ leftAnnotations && rightAnnotations &&
+ !leftAnnotations.filter( isForced ).compareTo(
rightAnnotations.filter( isForced ) )
+ ) {
+ return true;
+ }
+
+ return false;
};
/*! Getters */
diff --git a/modules/ve/dm/annotations/ve.dm.LinkAnnotation.js
b/modules/ve/dm/annotations/ve.dm.LinkAnnotation.js
index 433a162..28bdd6c 100644
--- a/modules/ve/dm/annotations/ve.dm.LinkAnnotation.js
+++ b/modules/ve/dm/annotations/ve.dm.LinkAnnotation.js
@@ -30,8 +30,6 @@
ve.dm.LinkAnnotation.static.matchTagNames = ['a'];
-ve.dm.LinkAnnotation.static.applyToAppendedContent = false;
-
ve.dm.LinkAnnotation.static.toDataElement = function ( domElements ) {
return {
'type': 'link',
--
To view, visit https://gerrit.wikimedia.org/r/73902
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id6424af89c92bba2be87736e8a937e0f2067c007
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Trevor Parscal <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
