jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/348708 )
Change subject: CX2: Fix broken relation between source and target sections in
translation units
......................................................................
CX2: Fix broken relation between source and target sections in translation units
* When there is translatable item in target section, but not in source,
it was not considered as a translation unit. For example, a link present in
translation section was skipped from translation unit instances. This commit
make sure that there is a translation unit even if a corresponding source does
not exist.
* Guarantee that multiple instances of translation units are not created for
same element. A key mechanism was introduced, which is calculated as same for
source and target. A map is maintained with this key and duplicates are
avoided.
This helps avoiding resetting the translation units array when a target
document
is set for a section(may be from restoring). This key calculation need some
refactoring in another iteration
* In UI models, fix broken relation between sourceSection and translationSection
This also need a clean up in another iteration.
* Also invalidate the assumption that a translation unit will always have source
document.
Testplan:
* Translate some sections. Check if the link pair, reference pair,
sentence pair highlighting is as expected.
* Select a word in translation section, from the link card, click
Add link. The new link should be added to current selection.
Clicking on that link will not have any effect for now(updating
model and rebuilding UI models in another commit). Save and reload
the page. The newly added link should have highlighting. When clicked
it should bring up the link card.
Bug: T163184
Change-Id: I3033799c623db712ea691075b1efd58f30c4e5a9
---
M modules/dm/translationunits/mw.cx.dm.LinkTranslationUnit.js
M modules/dm/translationunits/mw.cx.dm.TranslationUnit.js
M modules/tools/mw.cx.tools.NewLinkTool.js
M modules/tools/mw.cx.tools.SourceLinkTool.js
M modules/tools/mw.cx.tools.TargetLinkTool.js
M modules/ui/translationunits/mw.cx.ui.LinkTranslationUnit.js
M modules/ui/translationunits/mw.cx.ui.ReferenceTranslationUnit.js
M modules/ui/translationunits/mw.cx.ui.SectionTranslationUnit.js
M modules/ui/translationunits/mw.cx.ui.TranslationUnit.js
9 files changed, 181 insertions(+), 72 deletions(-)
Approvals:
jenkins-bot: Verified
Nikerabbit: Looks good to me, approved
diff --git a/modules/dm/translationunits/mw.cx.dm.LinkTranslationUnit.js
b/modules/dm/translationunits/mw.cx.dm.LinkTranslationUnit.js
index 4224d63..aa93b0c 100644
--- a/modules/dm/translationunits/mw.cx.dm.LinkTranslationUnit.js
+++ b/modules/dm/translationunits/mw.cx.dm.LinkTranslationUnit.js
@@ -26,16 +26,12 @@
mw.cx.dm.LinkTranslationUnit.static.matchTagNames = [ 'a' ];
mw.cx.dm.LinkTranslationUnit.static.matchRdfaTypes = [ 'mw:WikiLink' ];
-/**
- * @inheritDoc
- */
-mw.cx.dm.LinkTranslationUnit.static.matchFunction = function ( node ) {
- // Links should have id
- return !!node.id;
-};
-
mw.cx.dm.LinkTranslationUnit.prototype.init = function () {
- this.sourceTitle = this.sourceDocument.title;
+ if ( this.sourceDocument ) {
+ this.sourceTitle = this.sourceDocument.title;
+ } else {
+ mw.log( '[CX] Link translation unit without a source link: ' +
this.getTargetTitle() );
+ }
// We are not fetching any data before the parent translation unit's
translation started.
};
@@ -63,6 +59,7 @@
this.requestManager.getTitlePair( this.sourceLanguage, this.sourceTitle
)
.done( function( pairInfo ) {
var targetTitle = pairInfo.targetTitle;
+
if ( !targetTitle ) {
result.reject();
} else {
@@ -133,16 +130,35 @@
* Get the id of the section
* @return {string}
*/
-mw.cx.dm.TranslationUnit.prototype.getSectionId = function () {
+mw.cx.dm.LinkTranslationUnit.prototype.getSectionId = function () {
+ var id;
// Make sure that there is an id for the unit even if id attribute is
not present.
- return this.sourceDocument.id || this.sourceDocument.dataset.linkid ||
OO.ui.generateElementId();
+ if ( this.sourceDocument ) {
+ id = this.sourceDocument.id ||
this.sourceDocument.dataset.linkid ||
+ ( this.sourceDocument.attributes[ 'href' ] &&
this.sourceDocument.attributes[ 'href' ].value );
+ }
+ // Source document does not exist. See if there is target document
+ if ( !id && this.targetDocument ) {
+ id = this.targetDocument.id ||
this.targetDocument.dataset.linkid ||
+ ( this.targetDocument.attributes[ 'href' ] &&
this.targetDocument.attributes[ 'href' ].value );
+ }
+
+ return id || OO.ui.generateElementId();
};
/**
* @return {boolean} Whether a corresponding title exist in target language
*/
-mw.cx.dm.TranslationUnit.prototype.isTargetExist = function () {
+mw.cx.dm.LinkTranslationUnit.prototype.isTargetExist = function () {
return !this.targetTitleMissing;
};
+/**
+ * Remove the link. Removes only from the target document.
+ */
+mw.cx.dm.LinkTranslationUnit.prototype.removeLink = function () {
+ this.targetDocument.replaceWith( this.getTargetTitle() );
+ this.targetDocument = null;
+};
+
mw.cx.dm.modelRegistry.register( mw.cx.dm.LinkTranslationUnit );
diff --git a/modules/dm/translationunits/mw.cx.dm.TranslationUnit.js
b/modules/dm/translationunits/mw.cx.dm.TranslationUnit.js
index bba522d..b3c0300 100644
--- a/modules/dm/translationunits/mw.cx.dm.TranslationUnit.js
+++ b/modules/dm/translationunits/mw.cx.dm.TranslationUnit.js
@@ -79,46 +79,92 @@
if ( !this.targetDocument ) {
throw new Error( 'Target document not set for translation unit
: ' + this.toString() );
}
- this.targetDocument.id = 'cx' + this.sourceDocument.id;
+ this.targetDocument.id = this.getTranslationSectionId();
};
/**
* Section is the top most translation unit. Build and initialize its sub
translation units.
*
- * @param {Element} sourceDocument Source section DOM Element
+ * @param {Element} [sourceDocument] Source section DOM Element
* @param {Element} [targetDocument] Target section DOM Element
*/
mw.cx.dm.TranslationUnit.prototype.buildSubTranslationUnits = function (
sourceDocument, targetDocument ) {
- var i, model, children, targetChild, subTranslationUnit;
+ var i, sourceChildren, targetChildren;
- children = sourceDocument.children;
+ if ( !sourceDocument && !targetDocument ) {
+ throw new Error( '[CX] Both source and target documents can not
be null' );
+ }
- if ( !children ) {
+ if ( sourceDocument ) {
+ sourceChildren = sourceDocument.children || [];
+ for ( i = 0; i < sourceChildren.length; i++ ) {
+ this.buildSubTranslationUnit( sourceChildren[ i ] );
+ // Recursively search for sub translation units.
+ this.buildSubTranslationUnits( sourceChildren[ i ] );
+ }
+ }
+
+ if ( targetDocument ) {
+ // Scan target document
+ targetChildren = targetDocument.children || [];
+ for ( i = 0; i < targetChildren.length; i++ ) {
+ this.buildSubTranslationUnit( null, targetChildren[ i ]
);
+ // Recursively search for sub translation units.
+ this.buildSubTranslationUnits( null, targetChildren[ i
] );
+ }
+ }
+};
+
+/**
+ * Build a sub translation unit for given source, target element pair
+ *
+ * @param {Element} [source] Source DOM Element
+ * @param {Element} [target] Target DOM Element
+ */
+mw.cx.dm.TranslationUnit.prototype.buildSubTranslationUnit = function (
source, target ) {
+ var key, subTranslationUnit, model;
+
+ key = this.getKeyForModelMap( source || target );
+
+ model = mw.cx.dm.modelRegistry.matchElement( source || target );
+ // Check if there is a translation unit defined for this child
+ if ( !model ) {
return;
}
-
- for ( i = 0; i < children.length; i++ ) {
- model = mw.cx.dm.modelRegistry.matchElement( children[ i ] );
- // Check if there is a translation unit defined for this child
- if ( model ) {
- subTranslationUnit = this.subTranslationUnitModels[
children[ i ].id ] ||
- mw.cx.dm.translationUnitFactory.create(
- model, this.config, this.translation,
children[ i ]
- );
- if ( targetDocument ) {
- targetChild = targetDocument.querySelector(
'[id="' + subTranslationUnit.getTranslationSectionId() + '"]' );
- if ( targetChild ) {
- subTranslationUnit.setTargetDocument(
targetChild );
- }
- }
- // Keep a map of DOM ids and translation units
- this.subTranslationUnitModels[
subTranslationUnit.getSectionId() ] = subTranslationUnit;
- this.translationUnits.push( subTranslationUnit );
- subTranslationUnit.setParentTranslationUnit( this );
- }
- // Recursively search for sub translation units.
- this.buildSubTranslationUnits( children[ i ] );
+ if ( !key ) {
+ throw new Error( '[CX] Could not find any unique key for sub
translation unit of: ' + this.toString() );
}
+
+ subTranslationUnit = this.subTranslationUnitModels[ key ];
+ if ( subTranslationUnit ) {
+ subTranslationUnit.setTargetDocument( target );
+ } else {
+ if ( !source ) {
+ mw.log( '[CX] No source element for ' + target.id );
+ }
+ subTranslationUnit = mw.cx.dm.translationUnitFactory.create(
+ model, this.config, this.translation, source, target
+ );
+ // Keep a map of DOM ids and translation units
+ this.subTranslationUnitModels[ key ] = subTranslationUnit;
+ this.translationUnits.push( subTranslationUnit );
+ subTranslationUnit.setParentTranslationUnit( this );
+ }
+};
+
+/**
+ * Get a key for translation units based on id or such unique attributes.
+ * TODO: This id calculation is also present in multiple translation unit dms.
+ * Consolidate?
+ *
+ * @param {Element} node
+ * @return {string}
+ */
+mw.cx.dm.TranslationUnit.prototype.getKeyForModelMap = function ( node ) {
+ var id = node.id || node.dataset.linkid || node.dataset.segmentid ||
+ ( node.attributes[ 'about' ] && node.attributes[ 'about'
].value ) ||
+ ( node.attributes[ 'href' ] && node.attributes[ 'href' ].value
);
+ return id && id.replace( /^(cx)/, '' );
};
/**
@@ -136,8 +182,6 @@
mw.cx.dm.TranslationUnit.prototype.setTargetDocument = function (
targetDocument ) {
this.targetDocument = targetDocument;
- // The current units became invalid
- this.translationUnits = [];
this.buildSubTranslationUnits( this.sourceDocument, this.targetDocument
);
};
@@ -194,6 +238,10 @@
* @return {string}
*/
mw.cx.dm.TranslationUnit.prototype.getTranslationSectionId = function () {
+ if ( this.getSectionId().indexOf( 'cx' ) === 0 ) {
+ return this.getSectionId();
+ }
+
return 'cx' + this.getSectionId();
};
diff --git a/modules/tools/mw.cx.tools.NewLinkTool.js
b/modules/tools/mw.cx.tools.NewLinkTool.js
index 69b31c0..bd4a20b 100644
--- a/modules/tools/mw.cx.tools.NewLinkTool.js
+++ b/modules/tools/mw.cx.tools.NewLinkTool.js
@@ -103,9 +103,7 @@
if ( this.pageInfo && this.pageInfo.imageUrl ) {
return this.pageInfo.imageUrl;
}
- if ( !this.model.isTargetExist() ) {
- return;
- }
+ return null;
};
mw.cx.tools.NewLinkTool.prototype.removeLink = function () {
diff --git a/modules/tools/mw.cx.tools.SourceLinkTool.js
b/modules/tools/mw.cx.tools.SourceLinkTool.js
index 8a7e638..4a99d53 100644
--- a/modules/tools/mw.cx.tools.SourceLinkTool.js
+++ b/modules/tools/mw.cx.tools.SourceLinkTool.js
@@ -30,6 +30,10 @@
this.sourceTitle = this.model.sourceTitle;
+ if ( !this.sourceTitle ) {
+ return null;
+ }
+
$linkTitle = $( '<a>' )
.addClass( 'cx-tools-link-text' )
.text( this.sourceTitle )
@@ -67,6 +71,7 @@
if ( this.pageInfo && this.pageInfo.imageUrl ) {
return this.pageInfo.imageUrl;
}
+ return null;
};
/* Register */
diff --git a/modules/tools/mw.cx.tools.TargetLinkTool.js
b/modules/tools/mw.cx.tools.TargetLinkTool.js
index 8fb5fa3..3a49c93 100644
--- a/modules/tools/mw.cx.tools.TargetLinkTool.js
+++ b/modules/tools/mw.cx.tools.TargetLinkTool.js
@@ -177,18 +177,16 @@
if ( this.pageInfo && this.pageInfo.imageUrl ) {
return this.pageInfo.imageUrl;
}
- if ( !this.model.isTargetExist() ) {
- return;
- }
+ return null;
};
mw.cx.tools.TargetLinkTool.prototype.removeLink = function () {
- this.emit( 'remove' );
+ this.emit( 'removelink' );
this.destroy();
};
mw.cx.tools.TargetLinkTool.prototype.addLink = function () {
- this.emit( 'addlink', this.selectionObj, this.pageInfo.title );
+ this.emit( 'addlink', this.selectionObj, this.pageInfo &&
this.pageInfo.title );
};
/* Register */
diff --git a/modules/ui/translationunits/mw.cx.ui.LinkTranslationUnit.js
b/modules/ui/translationunits/mw.cx.ui.LinkTranslationUnit.js
index 1f589c8..64c3b8b 100644
--- a/modules/ui/translationunits/mw.cx.ui.LinkTranslationUnit.js
+++ b/modules/ui/translationunits/mw.cx.ui.LinkTranslationUnit.js
@@ -12,7 +12,7 @@
mw.cx.ui.LinkTranslationUnit.super.call( this, model, toolFactory,
config );
// Properties
- this.adapted = false;
+ this.adaptReq = null;
};
/* Setup */
@@ -24,29 +24,30 @@
mw.cx.ui.LinkTranslationUnit.static.highlightClass = 'cx-highlight--lightblue';
mw.cx.ui.LinkTranslationUnit.static.tools = {
sourcelink: [ 'click', 'focus' ],
- targetlink: [ 'click', 'focus' ],
+ targetlink: {
+ triggers: [ 'click', 'focus' ],
+ events: {
+ removelink: 'removeLink'
+ }
+ },
newlink: [ 'click', 'focus' ]
};
-/**
- * @inheritDoc
- */
-mw.cx.ui.LinkTranslationUnit.static.matchFunction = function ( node ) {
- // Links should have id
- return !!node.id;
-};
-
mw.cx.ui.LinkTranslationUnit.prototype.adapt = function () {
- var self = this;
+ if ( this.adaptReq ) {
+ return this.adaptReq;
+ }
// Adapt in general will be asynchronous operation
- this.model.adapt().then( function() {
- if ( !self.model.targetTitleMissing ) {
- self.adapted = true;
- self.setContent( self.model.targetDocument );
- } else {
- self.markUnAdapted();
+ this.adaptReq = this.model.adapt().then( function() {
+ this.setContent( this.model.targetDocument );
+ if ( this.model.targetTitleMissing ) {
+ this.markUnAdapted();
}
- } );
+ }.bind( this ) ).always( function() {
+ // Re attach event handlers
+ this.listen();
+ this.emit( 'change' );
+ }.bind( this ) );
};
mw.cx.ui.LinkTranslationUnit.prototype.markUnAdapted = function () {
@@ -63,12 +64,10 @@
$.each( attributes, function() {
self.$translationSection.attr( this.name, this.value );
} );
+ this.$translationSection.prop( 'id', $( content ).prop( 'id' ) );
// Refresh reference
this.$translationSection = this.getTranslationSection();
this.translated = true;
- // Re attach event handlers
- this.listen();
- this.emit( 'change' );
};
/**
@@ -81,5 +80,21 @@
}
};
+mw.cx.ui.LinkTranslationUnit.prototype.removeLink = function () {
+ // Save the selection
+ mw.cx.selection.save( 'translation' );
+ this.model.removeLink();
+
+ if ( this.model.getTargetDocument() === null ) {
+ mw.log( '[CX] Target link removed successfully. ' + this );
+ } else {
+ mw.log.error( '[CX] Error while removing target link. ' + this
);
+ }
+ this.parentTranslationUnit.focus();
+ // Restore the cursor
+ mw.cx.selection.restore( 'translation' );
+ this.emit( 'change' );
+};
+
/* Register */
mw.cx.ui.translationUnitFactory.register( mw.cx.ui.LinkTranslationUnit );
diff --git a/modules/ui/translationunits/mw.cx.ui.ReferenceTranslationUnit.js
b/modules/ui/translationunits/mw.cx.ui.ReferenceTranslationUnit.js
index 742d3a7..41d5ad7 100644
--- a/modules/ui/translationunits/mw.cx.ui.ReferenceTranslationUnit.js
+++ b/modules/ui/translationunits/mw.cx.ui.ReferenceTranslationUnit.js
@@ -40,12 +40,12 @@
return;
}
// Refresh reference
- this.$translationSection = this.getTranslationSection();
attributes = $( content ).prop( 'attributes' );
// loop through attributes and apply them.
$.each( attributes, function() {
self.$translationSection.attr( this.name, this.value );
} );
+ this.$translationSection.prop( 'id', $( content ).prop( 'id' ) );
// Refresh reference
this.$translationSection = this.getTranslationSection();
this.translated = true;
diff --git a/modules/ui/translationunits/mw.cx.ui.SectionTranslationUnit.js
b/modules/ui/translationunits/mw.cx.ui.SectionTranslationUnit.js
index be0a858..0064ccb 100644
--- a/modules/ui/translationunits/mw.cx.ui.SectionTranslationUnit.js
+++ b/modules/ui/translationunits/mw.cx.ui.SectionTranslationUnit.js
@@ -155,7 +155,25 @@
}
};
-mw.cx.ui.SectionTranslationUnit.prototype.addLink = function () {
+mw.cx.ui.SectionTranslationUnit.prototype.addLink = function ( selection,
title ) {
+ var $link;
+
+ mw.log( '[CX] Adding Link ' + title );
+ // Restore the selection
+ mw.cx.selection.restore( 'translation' );
+ $link = $( '<a>' )
+ .addClass( 'cx-target-link' )
+ .text( title )
+ .attr( {
+ title: title,
+ href: title,
+ rel: 'mw:WikiLink',
+ id: 'cx' + title
+ } );
+ mw.cx.selection.pasteHTML( $link[ 0 ].outerHTML );
+ // Set the cursor at the end of inserted link.
+ mw.cx.selection.setCursorAfter( 'translation' );
+ this.emit( 'change' );
};
/* Register */
diff --git a/modules/ui/translationunits/mw.cx.ui.TranslationUnit.js
b/modules/ui/translationunits/mw.cx.ui.TranslationUnit.js
index 9f5135c..8ba150e 100644
--- a/modules/ui/translationunits/mw.cx.ui.TranslationUnit.js
+++ b/modules/ui/translationunits/mw.cx.ui.TranslationUnit.js
@@ -60,6 +60,12 @@
};
mw.cx.ui.TranslationUnit.prototype.listen = function () {
+ if ( !this.$translationSection.length ) {
+ mw.log.warn( '[CX] Missing translation section for ' +
this.toString() );
+ }
+ if ( !this.$sourceSection.length ) {
+ mw.log.warn( '[CX] Missing source section for ' +
this.toString() );
+ }
// Events
this.$sourceSection.on( 'mouseenter', this.onMouseOver.bind( this ) );
this.$translationSection.on( 'mouseenter', this.onMouseOver.bind( this
) );
@@ -105,6 +111,11 @@
this.emit( 'focus' );
};
+mw.cx.ui.TranslationUnit.prototype.focus = function () {
+ this.getTranslationSection().focus();
+ this.emit( 'focus' );
+};
+
/**
* @private
* @return {mw.cx.TranslationTools[]}
--
To view, visit https://gerrit.wikimedia.org/r/348708
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3033799c623db712ea691075b1efd58f30c4e5a9
Gerrit-PatchSet: 12
Gerrit-Project: mediawiki/extensions/ContentTranslation
Gerrit-Branch: master
Gerrit-Owner: Santhosh <[email protected]>
Gerrit-Reviewer: Nikerabbit <[email protected]>
Gerrit-Reviewer: Santhosh <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
