Page MenuHomePhabricator

Changes made using NWE are blocked by Translate extension but copied into OWE they're accepted
Closed, ResolvedPublic8 Estimated Story Points


From T153445#2884971:

OK, here is the scenario:

  1. Opt in NWE.
  2. Open
  3. Open edit mode.
  4. Change something, for example remove a space after bullet.
  5. Try to save.
  6. You'll get a message "your editing was abandoned by Translation Extension. Click here to continue editing."
  7. Close the tab.
  8. Opt out NWE.
  9. Open the same page.
  10. Do the same edit.
  11. It works.

Naturally, there should be a way to edit translatable pages source - somebody should write them.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 3 2017, 5:58 PM
  1. I believe Translate is using the proper ways to propagate an error message, but that is not shown by VE.
  1. Investingating what is the issue, I find pt-shake-position which basically means that <!--T:#--> is in unexpected place. Inspecting the payload that is sent, I don't see why this would be the case.

Relevant code

				$rer1 = '~^<!--T:(.*?)-->( |\n)~'; // Normal sections
				$rer2 = '~\s*<!--T:(.*?)-->$~m'; // Sections with title
				$content = preg_replace( $rer1, '', $content );
				$content = preg_replace( $rer2, '', $content );

				if ( preg_match( $re, $content ) === 1 ) {
					throw new TPException( array( 'pt-shake-position', $content ) );

My guess would be that the line feeds sent by NWE are different (\r\n perhaps?) and it does not get normalized.

Seems a good guess to explore.

Since I could not figure how to access old wikitext editor easily anymore, another workaround is to do the edit in the source editor, but switch to visual editor before saving.

Gnom1 added a subscriber: Gnom1.Jun 8 2017, 2:12 PM

The error message I'm getting in this scenario reads, "Multiple translation unit markers for one translation unit. Translation unit text: [...]". Making the same edit in OWE also works fine. Or is this a different bug?

It's not exactly the same error, but most likely it is the same bug.

Screenshots and examples, in case that helps:

I can confirm my guess:

string(1482) "ApiMain->execute/ApiMain->executeActionWithErrorHandling/ApiMain->executeAction/ApiVisualEditorEdit->execute/ApiVisualEditorEdit->saveWikitext/ApiMain->execute/ApiMain->executeAction/ApiEditPage->execute/EditPage->attemptSave/EditPage->internalAttemptSave/EditPage->runPostMergeFilters/Hooks::run/Hooks::callHook/AbuseFilterHooks::onEditFilterMergedContent/AbuseFilterHooks::filterEdit/AbuseFilter::filterAction/AbuseFilter::checkAllFilters/AbuseFilter::checkFilter/AbuseFilter::checkConditions/AbuseFilterParser->parse/AbuseFilterParser->intEval/AbuseFilterParser->doLevelEntry/AbuseFilterParser->doLevelSemicolon/AbuseFilterParser->doLevelSet/AbuseFilterParser->doLevelConditions/AbuseFilterParser->doLevelBoolOps/AbuseFilterParser->doLevelCompares/AbuseFilterParser->doLevelSumRels/AbuseFilterParser->doLevelMulRels/AbuseFilterParser->doLevelPow/AbuseFilterParser->doLevelBoolInvert/AbuseFilterParser->doLevelSpecialWords/AbuseFilterParser->doLevelUnarys/AbuseFilterParser->doLevelListElements/AbuseFilterParser->doLevelBraces/AbuseFilterParser->doLevelFunction/AbuseFilterParser->doLevelAtom/AbuseFilterParser->getVarValue/AbuseFilterVariableHolder->getVar/AFComputedVariable->compute/AbuseFilterVariableHolder->getVar/AFComputedVariable->compute/WikiPage->prepareContentForEdit/AbstractContent->getParserOutput/WikitextContent->fillParserOutput/Parser->parse/Hooks::run/Hooks::callHook/PageTranslationHooks::renderTagPage/TranslatablePage->getParse/TranslatablePage::sectionise"
string(67) "%0A%3C%21--T%3A1--%3E%0AHello%2C%0A%0A%3C%21--T%3A2--%3E%0AWorld%0A"
string(59) "["\n","<!--T:1-->\nHello,","\n\n","<!--T:2-->\nWorld","\n"]"
string(454) "ApiMain->execute/ApiMain->executeActionWithErrorHandling/ApiMain->executeAction/ApiVisualEditorEdit->execute/ApiVisualEditorEdit->saveWikitext/ApiMain->execute/ApiMain->executeAction/ApiEditPage->execute/EditPage->attemptSave/EditPage->internalAttemptSave/EditPage->runPostMergeFilters/Hooks::run/Hooks::callHook/PageTranslationHooks::tpSyntaxCheckForEditContent/PageTranslationHooks::tpSyntaxError/TranslatablePage->getParse/TranslatablePage::sectionise"
string(85) "%0D%0A%3C%21--T%3A1--%3E%0D%0AHello%2C%0D%0A%0D%0A%3C%21--T%3A2--%3E%0D%0AWorld%0D%0A"
string(65) "["\r\n","<!--T:1-->\r\nHello,\r\n\r\n<!--T:2-->\r\nWorld","\r\n"]"

Interestingly the first one succeeds, so normalization is happening at some point.

Common prefix is ApiMain->execute/ApiMain->executeActionWithErrorHandling/ApiMain->executeAction/ApiVisualEditorEdit->execute/ApiVisualEditorEdit->saveWikitext/ApiMain->execute/ApiMain->executeAction/ApiEditPage->execute/EditPage->attemptSave/EditPage->internalAttemptSave/EditPage->runPostMergeFilters/Hooks::run/Hooks::callHook/

AbuseFilter::contentToString does replacing of newlines, so that is the reason that works. Also TextContent::normalizeLineEndings does this in preSaveTransform, but it seems to happen after EditFilterMergedContent is called. I think sane option would be to fix this, rather than risk introducing bugs in all extensions using this hook.

Update: apparently the newlines are still broken even in PageContentSave hook. Makes me wonder if TextContent::preSaveTransform is called at all.

Change 383330 had a related patch set uploaded (by Nikerabbit; owner: Nikerabbit):
[mediawiki/extensions/Translate@master] Work around core's lack of new line normalization

I consider my fix only a temporary workaround and would hope this normalization is handled by MediaWiki core before hooks are called.

Change 383330 merged by jenkins-bot:
[mediawiki/extensions/Translate@master] Work around core's lack of new line normalization

The fix seems to be working. I used the text content from in meta.betalabs (Extension:Translate is enabled there) and made small edits with "New wikitext mode" beta feature enabled. The edits have been successfully saved.

Etonkovidova closed this task as Resolved.Nov 17 2017, 2:47 AM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptNov 17 2017, 2:47 AM
Nikerabbit moved this task from QA to Done on the Language-2017-Oct-Dec board.Nov 19 2017, 7:50 PM