jenkins-bot has submitted this change and it was merged.
Change subject: Corrections for draft restore based on permanent ids
......................................................................
Corrections for draft restore based on permanent ids
* The translation column may not be ready when draft module initializes
this.$translation because draft module is loaded in parallel
with source.
This can cause incorrect draft restore. Fixed that.
* Update the data-source attributes correctly in draft restore.
Icc5dff9ba5cf79c599021f2a633024a257b397fb missed to update the
data-source attributes when sections are restored.
This causes wrong progress calculation.
* Section alignment was not working for sections restored in linear
order or even for the old draft sections because the
keepAlignment plugin was not reading the updated data-source value.
That plugin was slightly modified to read up-to-date
data-source values.
* Updated the tests accordingly.
Testplan:
Run a local cxserver without 92cdf4ccaf1b4a21afe9d6675c4549187924f707
and MWCX without f2c8517d59c4eaee0f74589c91a069b9eb4ba5dd.
Create some draft translations, then update cxserver and MWCX
to this commit. Make sure you can retrieve your drafts,
and all section alignments are proper.
Change-Id: I6406e43cbfef1c83f6f0de7b6fe9874a20f43d60
---
M modules/draft/ext.cx.draft.js
M modules/translation/ext.cx.translation.aligner.js
M tests/qunit/draft/ext.cx.draft.test.js
3 files changed, 62 insertions(+), 30 deletions(-)
Approvals:
Amire80: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/draft/ext.cx.draft.js b/modules/draft/ext.cx.draft.js
index 9424f13..3f4ea05 100644
--- a/modules/draft/ext.cx.draft.js
+++ b/modules/draft/ext.cx.draft.js
@@ -19,8 +19,8 @@
this.draftId = draftId;
this.$draft = null;
this.disabled = false;
- this.$source = $( '.cx-column--source .cx-column__content' );
- this.$translation = $( '.cx-column--translation
.cx-column__content' );
+ this.$source = null;
+ this.$translation = null;
this.init();
this.listen();
}
@@ -44,6 +44,9 @@
ContentTranslationDraft.prototype.getContent = function () {
var $content;
+ if ( !this.$translation || !this.$translation.length ) {
+ this.$translation = $( '.cx-column--translation
.cx-column__content' );
+ }
$content = this.$translation.clone();
// Remove placeholder sections
$content.find( '.placeholder' ).remove();
@@ -169,11 +172,11 @@
* @param {jQuery} $section Add it before/after this section.
* @param {string} afterOrBefore Whether the orphan to be added after
or before $section.
*/
- function addOrphanTranslation( $translation, $section, afterOrBefore ) {
+ ContentTranslationDraft.prototype.addOrphanTranslation = function (
$translation, $section, afterOrBefore ) {
// Add a dummy source section
var $dummySourceSection = $( '<' + $translation.prop( 'tagName'
) + '>' )
.css( 'height', 1 ) // Non-zero height to avoid it
ignored by keepAlignment plugin.
- .prop( 'id', $translation.data( 'source' ) );
+ .attr( 'id', $translation.data( 'source' ) );
if ( afterOrBefore === 'after' ) {
$( '#' + $section.data( 'source' ) ).after(
$dummySourceSection );
@@ -184,29 +187,48 @@
}
// Annotate the section to indicate it was restored from draft
// so that certain adaptations can be skipped.
- $translation.attr( 'data-cx-draft', true ).keepAlignment();
+ $translation.attr( {
+ id: 'cx' + $dummySourceSection.prop( 'id' ),
+ 'data-cx-draft': true,
+ 'data-source': $dummySourceSection.prop( 'id' )
+ } ).keepAlignment();
mw.hook( 'mw.cx.translation.postMT' ).fire( $translation );
- }
+ };
/**
* Restore this draft to the appropriate placeholders
*/
ContentTranslationDraft.prototype.restore = function () {
- var i, j, $draftSection, sectionId, $section,
+ var i, j, $draftSection, sectionId, $section, $sourceSection,
$placeholderSection, orphans = [],
$lastPlaceholder;
+ // We cannot populate this early because this ext.cx.draft
modules may be loaded before
+ // source and target is ready.
+ if ( !this.$source || !this.$source.length ) {
+ this.$source = $( '.cx-column--source
.cx-column__content' );
+ }
+ if ( !this.$translation || !this.$translation.length ) {
+ this.$translation = $( '.cx-column--translation
.cx-column__content' );
+ }
for ( i = 0; i < this.$draft.length; i++ ) {
$draftSection = $( this.$draft[ i ] );
sectionId = $draftSection.prop( 'id' );
+ if ( !sectionId ) {
+ // Is this possible?
+ continue;
+ }
$placeholderSection = this.$translation.find( '#' +
sectionId );
+ $sourceSection = this.$source.find( '#' +
$placeholderSection.data( 'source' ) );
if ( !$placeholderSection.length ) {
// Support old sections with sequential
idendifiers
+ $sourceSection = this.$source.find(
'[data-seqid="' + $draftSection.data( 'source' ) + '"]' );
$placeholderSection = this.$translation.find(
'#cx' +
- this.$source.find( '[data-seqid="' +
$draftSection.data( 'source' ) + '"]' ).prop( 'id' )
+ $sourceSection.prop( 'id' )
);
sectionId = $placeholderSection.prop( 'id' );
if ( sectionId ) {
+ // Update the id of this old draft
$draftSection.prop( 'id', sectionId );
}
}
@@ -223,12 +245,15 @@
$section = this.$translation.find( '#' + sectionId );
// Annotate the section to indicate it was restored
from draft
// so that certain adaptations can be skipped.
- $section.attr( 'data-cx-draft', true ).keepAlignment();
+ $section.attr( {
+ 'data-cx-draft': true,
+ 'data-source': $sourceSection.prop( 'id' )
+ } ).keepAlignment();
mw.hook( 'mw.cx.translation.postMT' ).fire( $section );
// We have a matching source and target section. Get
all orphan backlog added.
// We add them before this section.
for ( j = 0; j < orphans.length; j++ ) {
- addOrphanTranslation( orphans[ j ], $section );
+ this.addOrphanTranslation( orphans[ j ],
$section );
}
// Clear the orphans array
orphans = [];
@@ -242,13 +267,18 @@
$placeholderSection = this.$translation.find(
'.placeholder:first' );
if ( $placeholderSection &&
$placeholderSection.length ) {
sectionId = orphans[ j ].prop( 'id' );
+ $sourceSection = this.$source.find( '#'
+ $placeholderSection.data( 'source' ) );
$placeholderSection.replaceWith(
orphans[ j ] );
$section = this.$translation.find( '#'
+ sectionId );
- $section.attr( 'data-cx-draft', true
).keepAlignment();
+ $section.attr( {
+ id: 'cx' + $sourceSection.prop(
'id' ),
+ 'data-cx-draft': true,
+ 'data-source':
$sourceSection.prop( 'id' )
+ } ).keepAlignment();
mw.hook( 'mw.cx.translation.postMT'
).fire( $section );
} else {
// We ran out of placeholders. Add
orphan sections to end.
- addOrphanTranslation( orphans[ j ],
$section, 'after' );
+ this.addOrphanTranslation( orphans[ j
], $section, 'after' );
}
}
} else {
@@ -256,7 +286,7 @@
$lastPlaceholder = this.$translation.find(
'.placeholder:last' );
for ( j = 0; j < orphans.length; j++ ) {
// Add it after the last placeholder
- addOrphanTranslation( orphans[ j ],
$lastPlaceholder, 'after' );
+ this.addOrphanTranslation( orphans[ j ],
$lastPlaceholder, 'after' );
}
}
diff --git a/modules/translation/ext.cx.translation.aligner.js
b/modules/translation/ext.cx.translation.aligner.js
index 143beae..c00835b 100644
--- a/modules/translation/ext.cx.translation.aligner.js
+++ b/modules/translation/ext.cx.translation.aligner.js
@@ -22,7 +22,9 @@
steps = 0;
$section = $( this );
- $sourceSection = mw.cx.getSourceSection( $section.data(
'source' ) );
+ // Get the source section. We don't use $section.data('source')
because that
+ // won't reflect updated data-source values(probably from
section restore attempt)
+ $sourceSection = mw.cx.getSourceSection( $section.attr(
'data-source' ) );
if ( $section.is( '.placeholder' ) ) {
$section.css( {
diff --git a/tests/qunit/draft/ext.cx.draft.test.js
b/tests/qunit/draft/ext.cx.draft.test.js
index 5cc6f46..137f485 100644
--- a/tests/qunit/draft/ext.cx.draft.test.js
+++ b/tests/qunit/draft/ext.cx.draft.test.js
@@ -10,51 +10,51 @@
var tests = [ {
description: 'All ids of source and draft match
perfectly',
source: '<div><p id="mw1">Paragraph 1</p></div>',
- placeholders: '<div><div id="cxmw1"
class="placeholder"></div></div>',
+ placeholders: '<div><div id="cxmw1" data-source="mw1"
class="placeholder"></div></div>',
draft: '<p id="cxmw1">PARAGRAPH 1</p>',
- translation: '<p id="cxmw1"
data-cx-draft="true">PARAGRAPH 1</p>'
+ translation: '<p id="cxmw1" data-cx-draft="true"
data-source="mw1">PARAGRAPH 1</p>'
},
{
description: 'All ids of source and draft match
perfectly, Restoring old draft using sequence ids',
source: '<div><p id="mw1" data-seqid="1">Paragraph
1</p></div>',
placeholders: '<div><div id="cxmw1" data-source="mw1"
class="placeholder"></div></div>',
draft: '<p id="cx1" data-source="1">PARAGRAPH 1</p>',
- translation: '<p id="cxmw1" data-source="1"
data-cx-draft="true">PARAGRAPH 1</p>'
+ translation: '<p id="cxmw1" data-source="mw1"
data-cx-draft="true">PARAGRAPH 1</p>'
},
{
description: 'Source section replaced by new section.
Insert the draft above the placeholder for next match',
source: '<div><p id="mw2">Paragraph 2</p><p
id="mw3">Paragraph 3</p></div>',
- placeholders: '<div><div id="cxmw2"
class="placeholder"></div><div id="cxmw3" class="placeholder"></div></div>',
- draft: '<p id="cxmw1">PARAGRAPH 1</p><p
id="cxmw3">PARAGRAPH 3</p>',
- translation: '<div id="cxmw2"
class="placeholder"></div><p id="cxmw1" data-cx-draft="true">PARAGRAPH 1</p><p
id="cxmw3" data-cx-draft="true">PARAGRAPH 3</p>'
+ placeholders: '<div><div id="cxmw2" data-source="mw2"
class="placeholder"></div><div id="cxmw3" data-source="mw3"
class="placeholder"></div></div>',
+ draft: '<p id="cxmw1" data-source="mw1">PARAGRAPH
1</p><p id="cxmw3">PARAGRAPH 3</p>',
+ translation: '<div id="cxmw2" data-source="mw2"
class="placeholder"></div><p id="cxmw1" data-source="mw1"
data-cx-draft="true">PARAGRAPH 1</p><p id="cxmw3" data-cx-draft="true"
data-source="mw3">PARAGRAPH 3</p>'
},
{
description: 'Source section has new sections above and
below. Place the draft in correct position',
source: '<div><p id="mw1">Paragraph 1</p><div><p
id="mw2">Paragraph 2</p><p id="mw3">Paragraph 3</p></div>',
- placeholders: '<div><div id="cxmw1"
class="placeholder"></div><div id="cxmw2" class="placeholder"></div><div
id="cxmw3" class="placeholder"></div></div>',
+ placeholders: '<div><div id="cxmw1" data-source="mw1"
class="placeholder"></div><div id="cxmw2" data-source="mw2"
class="placeholder"></div><div id="cxmw3" data-source="mw3"
class="placeholder"></div></div>',
draft: '<p id="cxmw2">PARAGRAPH 2</p>',
- translation: '<div id="cxmw1"
class="placeholder"></div><p id="cxmw2" data-cx-draft="true">PARAGRAPH
2</p><div id="cxmw3" class="placeholder"></div>'
+ translation: '<div id="cxmw1" data-source="mw1"
class="placeholder"></div><p id="cxmw2" data-cx-draft="true"
data-source="mw2">PARAGRAPH 2</p><div id="cxmw3" data-source="mw3"
class="placeholder"></div>'
},
{
description: 'Source section has new order of sections.
Place the draft in correct position',
source: '<div><p id="mw3">Paragraph 3</p><div><p
id="mw2">Paragraph 2</p><p id="mw1">Paragraph 1</p></div>',
- placeholders: '<div><div id="cxmw3"
class="placeholder"></div><div id="cxmw2" class="placeholder"></div><div
id="cxmw1" class="placeholder"></div></div>',
+ placeholders: '<div><div id="cxmw3" data-source="mw3"
class="placeholder"></div><div id="cxmw2" data-source="mw2"
class="placeholder"></div><div id="cxmw1" data-source="mw1"
class="placeholder"></div></div>',
draft: '<p id="cxmw1">PARAGRAPH 1</p>',
- translation: '<div id="cxmw3"
class="placeholder"></div><div id="cxmw2" class="placeholder"></div><p
id="cxmw1" data-cx-draft="true">PARAGRAPH 1</p>'
+ translation: '<div id="cxmw3" data-source="mw3"
class="placeholder"></div><div id="cxmw2" data-source="mw2"
class="placeholder"></div><p id="cxmw1" data-cx-draft="true"
data-source="mw1">PARAGRAPH 1</p>'
},
{
description: 'Source article changed completely. Place
the draft in linear order from the beginning of placeholders',
source: '<div><p id="mw1">Paragraph 1</p><div><p
id="mw2">Paragraph 2</p><p id="mw3">Paragraph 3</p></div>',
- placeholders: '<div><div id="cxmw1"
class="placeholder"></div><div id="cxmw2" class="placeholder"></div><div
id="cxmw3" class="placeholder"></div></div>',
- draft: '<p id="cxmw4">PARAGRAPH 4</p><p
id="cxmw5">PARAGRAPH 5</p>',
- translation: '<p id="cxmw4"
data-cx-draft="true">PARAGRAPH 4</p><p id="cxmw5"
data-cx-draft="true">PARAGRAPH 5</p><div id="cxmw3" class="placeholder"></div>'
+ placeholders: '<div><div id="cxmw1" data-source="mw1"
class="placeholder"></div><div id="cxmw2" data-source="mw2"
class="placeholder"></div><div id="cxmw3" data-source="mw3"
class="placeholder"></div></div>',
+ draft: '<p id="cxmw4" data-source="mw4">PARAGRAPH
4</p><p id="cxmw5" data-source="mw5">PARAGRAPH 5</p>',
+ translation: '<p id="cxmw1" data-source="mw1"
data-cx-draft="true">PARAGRAPH 4</p><p id="cxmw2" data-source="mw2"
data-cx-draft="true">PARAGRAPH 5</p><div id="cxmw3" data-source="mw3"
class="placeholder"></div>'
},
{
description: 'Source article changed completely. More
draft sections than source sections. Place the draft in linear order from the
beginning of placeholders. Rest of them below the source article.',
source: '<div><p id="mw1">Paragraph 1</p><div><p
id="mw2">Paragraph 2</p><p id="mw3">Paragraph 3</p></div>',
- placeholders: '<div><div id="cxmw1"
class="placeholder"></div><div id="cxmw2" class="placeholder"></div><div
id="cxmw3" class="placeholder"></div></div>',
- draft: '<p id="cxmw4">PARAGRAPH 4</p><p
id="cxmw5">PARAGRAPH 5</p><p id="cxmw6">PARAGRAPH 6</p><p id="cxmw7">PARAGRAPH
7</p>',
- translation: '<p id="cxmw4"
data-cx-draft="true">PARAGRAPH 4</p><p id="cxmw5"
data-cx-draft="true">PARAGRAPH 5</p><p id="cxmw6"
data-cx-draft="true">PARAGRAPH 6</p><p id="cxmw7"
data-cx-draft="true">PARAGRAPH 7</p>'
+ placeholders: '<div><div id="cxmw1" data-source="mw1"
class="placeholder"></div><div id="cxmw2" data-source="mw2"
class="placeholder"></div><div id="cxmw3" data-source="mw3"
class="placeholder"></div></div>',
+ draft: '<p id="cxmw4">PARAGRAPH 4</p><p
id="cxmw5">PARAGRAPH 5</p><p id="cxmw6">PARAGRAPH 6</p><p id="cxmw7"
data-source="mw7">PARAGRAPH 7</p>',
+ translation: '<p id="cxmw1" data-cx-draft="true"
data-source="mw1">PARAGRAPH 4</p><p id="cxmw2" data-cx-draft="true"
data-source="mw2">PARAGRAPH 5</p><p id="cxmw3" data-cx-draft="true"
data-source="mw3">PARAGRAPH 6</p><p id="cxmw7" data-source="mw7"
data-cx-draft="true">PARAGRAPH 7</p>'
}
];
--
To view, visit https://gerrit.wikimedia.org/r/227940
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6406e43cbfef1c83f6f0de7b6fe9874a20f43d60
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/ContentTranslation
Gerrit-Branch: master
Gerrit-Owner: Santhosh <[email protected]>
Gerrit-Reviewer: Amire80 <[email protected]>
Gerrit-Reviewer: KartikMistry <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
