Page MenuHomePhabricator
Create Task
Maniphest T245811

PagePreparation doesn't detect languages tag already in page
Closed, ResolvedPublic
Actions

Assigned To
Reedy
Authored By
RhinosF1
Feb 21 2020, 10:15 AM
Tags
Referenced Files
F37149620: Screenshot 2023-07-25 at 19.18.16.png
Jul 25 2023, 6:19 PM
F37149601: Screenshot 2023-07-25 at 19.04.30.png
Jul 25 2023, 6:05 PM
Subscribers
Aklapper
Reception123
Reedy
RhinosF1

Description

When marking a page for translation using Special:PagePreparation:

If it has the following as the format:

<noinclude><languages /></noinclude>
//my page content//

Then it will become:

<languages/>
<translate>
<noinclude><languages /></noinclude>
//my page content//
</translate>

It should become:

<noinclude><languages /></noinclude>
<translate>
//my page content//
</translate>

Impact:

  1. 2 Language tags will be shown as in https://meta.miraheze.org/w/index.php?title=Training_modules/Dealing_with_online_harassment/slides/follow-up-tracking-and-appeals&diff=prev&oldid=97982
  2. The first translate section will be <noinclude><languages /></noinclude> which is untranslateable
MediaWiki Version1.34.0 (f0cbfa7) 02:15, 14 February 2020
Extension:Translate Version2020-01-23 (fa4d922) 16:09, 24 January 2020

Details

SubjectRepoBranchLines +/-
ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>mediawiki/extensions/TranslateREL1_35+7 -2
ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>mediawiki/extensions/TranslateREL1_39+7 -2
ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>mediawiki/extensions/TranslateREL1_40+7 -2
ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>mediawiki/extensions/Translatemaster+7 -2
ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> elementmediawiki/extensions/TranslateREL1_35+2 -2
ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> elementmediawiki/extensions/TranslateREL1_40+2 -2
ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> elementmediawiki/extensions/TranslateREL1_39+2 -2
ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> elementmediawiki/extensions/Translatemaster+2 -2
Customize query in gerrit

Related Objects

Event Timeline

RhinosF1 created this task.Feb 21 2020, 10:15 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 21 2020, 10:15 AM
RhinosF1 moved this task from Radar to Miraheze-Related Issues (not reported there) on the User-RhinosF1 board.Feb 21 2020, 10:15 AM
RhinosF1 moved this task from Miraheze-Related Issues (not reported there) to Miraheze-Linked on the User-RhinosF1 board.May 15 2020, 1:14 PM
Pginer-WMF edited projects, added MediaWiki-extensions-Translate; removed ContentTranslation.Mar 14 2022, 11:37 AM
Restricted Application added a subscriber: Reception123. · View Herald TranscriptMar 14 2022, 11:38 AM
Pginer-WMF triaged this task as Medium priority.Mar 28 2022, 11:25 AM
Pginer-WMF moved this task from Backlog to page migration on the MediaWiki-extensions-Translate board.
Aklapper edited projects, added affects-Miraheze; removed User-RhinosF1.Aug 15 2022, 1:19 PM
Reedy renamed this task from TranslatePreparation doesn't detect languages tag already in page to PagePreparation doesn't detect languages tag already in page.Jul 25 2023, 1:33 PM
Reedy updated the task description. (Show Details)
Reedy mentioned this in T342651: Special:PagePreparation should handled pages with translation markup better (or do nothing).Jul 25 2023, 1:38 PM
Reedy mentioned this in T342622: Clicking Prepare multiple times on Special:PagePreparation results in multiple diffs being appended.Jul 25 2023, 1:44 PM
Reedy subscribed.Jul 25 2023, 1:50 PM

The problem is that the regex doesn't look for any whitespace in the initial check...

	/**
	 * Add the <languages/> bar at the top of the page, if not present.
	 * Remove the old {{languages}} template, if present.
	 *
	 * @param {string} pageContent
	 * @return {string}
	 */
	function addLanguageBar( pageContent ) {
		if ( !/<languages\/>/gi.test( pageContent ) ) {
			pageContent = '<languages/>\n' + pageContent;
		}
		pageContent = pageContent.replace( /\{\{languages.*?\}\}/gi, '' );
		return pageContent;
	}
gerritbot added a comment.Jul 25 2023, 1:52 PM

Change 941415 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@master] ext.translate.special.pagepreparation.js: Check for whitespace in <language/> element

https://gerrit.wikimedia.org/r/941415

gerritbot added a project: Patch-For-Review.Jul 25 2023, 1:52 PM
Reedy added a comment.Jul 25 2023, 1:56 PM

I don't know whether we should be "standardising" <languages/> becoming <languages />; maybe we should...

Not quite sure what happens with the noincludes currently... Lets see.

gerritbot added a comment.Jul 25 2023, 6:05 PM

Change 941415 merged by jenkins-bot:

[mediawiki/extensions/Translate@master] ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> element

https://gerrit.wikimedia.org/r/941415

Reedy added a comment.Jul 25 2023, 6:05 PM

With the patch it results in

Screenshot 2023-07-25 at 19.04.30.png (214×2 px, 22 KB)

Maintenance_bot removed a project: Patch-For-Review.Jul 25 2023, 6:10 PM
gerritbot added a comment.Jul 25 2023, 6:13 PM

Change 941486 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@REL1_39] ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> element

https://gerrit.wikimedia.org/r/941486

gerritbot added a project: Patch-For-Review.Jul 25 2023, 6:13 PM

Change 941487 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@REL1_40] ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> element

https://gerrit.wikimedia.org/r/941487

gerritbot added a comment.Jul 25 2023, 6:14 PM

Change 941488 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@REL1_35] ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> element

https://gerrit.wikimedia.org/r/941488

gerritbot added a comment.Jul 25 2023, 6:16 PM

Change 941486 merged by jenkins-bot:

[mediawiki/extensions/Translate@REL1_39] ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> element

https://gerrit.wikimedia.org/r/941486

gerritbot added a comment.Jul 25 2023, 6:17 PM

Change 941487 merged by jenkins-bot:

[mediawiki/extensions/Translate@REL1_40] ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> element

https://gerrit.wikimedia.org/r/941487

Reedy added a comment.EditedJul 25 2023, 6:19 PM
In T245811#9042352, @Reedy wrote:

With the patch it results in

Screenshot 2023-07-25 at 19.04.30.png (214×2 px, 22 KB)

Not quite...

Screenshot 2023-07-25 at 19.18.16.png (200×2 px, 24 KB)

Why is it adding </translate> without <translate>...

gerritbot added a comment.Jul 25 2023, 6:31 PM

Change 941488 merged by jenkins-bot:

[mediawiki/extensions/Translate@REL1_35] ext.translate.special.pagepreparation.js: Check for whitespace in <languages/> element

https://gerrit.wikimedia.org/r/941488

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.20; 2023-08-01).Jul 25 2023, 7:00 PM
Maintenance_bot removed a project: Patch-For-Review.Jul 25 2023, 7:10 PM
Reedy added a comment.Jul 25 2023, 7:28 PM
In T245811#9042379, @Reedy wrote:

Screenshot 2023-07-25 at 19.18.16.png (200×2 px, 24 KB)

Why is it adding </translate> without <translate>...

	/**
	 * Add the <translate> and </translate> tags at the start and end of the page.
	 * The opening tag is added immediately after the <languages/> tag.
	 *
	 * @param {string} pageContent
	 * @return {string}
	 */
	function addTranslateTags( pageContent ) {
		pageContent = pageContent.replace( /(<languages ?\/>\n)/gi, '$1<translate>\n' );
		pageContent = pageContent + '\n</translate>';
		return pageContent;
	}

Because it's looking for the newline...

Not 100% clear what we should be doing here though, though a different bug to what was actually originally reported... Do we necessarily want to fix this?

Should we be looking for a newline? Swap the regex for /(<languages ?\/>\n?)/gi?

But then, in this case, for example, we end up with a translate tag inside a <noinclude>, something like:

<noinclude><languages /><translate></noinclude>
Text here
</translate>

Which is not right either.

If we can't add the <translate>, should we be adding the </translate> at the end? I suspect it won't save in either version anyway, so it's not helping the user at all.

Or should we skip it, and let the rest just do what it needs to do? Users could then manually add leading/trailing tags as appropriate.

Is this a common enough use case?

Certainly, this is commonly done in Templates...

<noinclude>
<languages/>
</noinclude>
Reedy mentioned this in T342695: Have a way to take text from Special:PagePreparation "prepared" page text straight into the edit screen.Jul 25 2023, 7:32 PM
gerritbot added a comment.Jul 25 2023, 7:33 PM

Change 941492 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@master] ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>

https://gerrit.wikimedia.org/r/941492

gerritbot added a project: Patch-For-Review.Jul 25 2023, 7:33 PM
gerritbot added a comment.Jul 26 2023, 4:42 AM

Change 941492 merged by jenkins-bot:

[mediawiki/extensions/Translate@master] ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>

https://gerrit.wikimedia.org/r/941492

Maintenance_bot removed a project: Patch-For-Review.Jul 26 2023, 5:10 AM
abi_ assigned this task to Reedy.Jul 26 2023, 7:05 AM
abi_ added projects: Localization Infrastructure FY2023-24, Language-Team (Language-2023-July-September).
abi_ moved this task from Quarter Backlog to Check after deployment on the Language-Team (Language-2023-July-September) board.
gerritbot added a comment.Jul 26 2023, 10:41 AM

Change 941867 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@REL1_40] ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>

https://gerrit.wikimedia.org/r/941867

gerritbot added a project: Patch-For-Review.Jul 26 2023, 10:41 AM

Change 941868 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@REL1_39] ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>

https://gerrit.wikimedia.org/r/941868

gerritbot added a comment.Jul 26 2023, 10:42 AM

Change 941870 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@REL1_35] ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>

https://gerrit.wikimedia.org/r/941870

gerritbot added a comment.Jul 26 2023, 10:44 AM

Change 941868 merged by jenkins-bot:

[mediawiki/extensions/Translate@REL1_39] ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>

https://gerrit.wikimedia.org/r/941868

gerritbot added a comment.Jul 26 2023, 10:44 AM

Change 941867 merged by jenkins-bot:

[mediawiki/extensions/Translate@REL1_40] ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>

https://gerrit.wikimedia.org/r/941867

Reedy closed this task as Resolved.EditedJul 26 2023, 10:46 AM

It doesn't result in what was actually requested to begin with, but it handles some of these cases better, and doesn't duplicate syntax either.

As per T245811#9042640

<noinclude>
<languages/>
</noinclude>

should probably be used, rather than

<noinclude><languages/></noinclude>
gerritbot added a comment.Jul 26 2023, 10:47 AM

Change 941870 merged by jenkins-bot:

[mediawiki/extensions/Translate@REL1_35] ext.translate.special.pagepreparation.js: Only add trailing </translate> if we added a leading <translate>

https://gerrit.wikimedia.org/r/941870

Maintenance_bot removed a project: Patch-For-Review.Jul 26 2023, 11:11 AM
Nikerabbit moved this task from Check after deployment to Done on the Language-Team (Language-2023-July-September) board.Jul 31 2023, 12:15 PM
Nikerabbit moved this task from Backlog to Translatable pages (LH1) on the Localization Infrastructure FY2023-24 board.
Nikerabbit added a project: Unplanned-Sprint-Work.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL