Page MenuHomePhabricator

Edit conflicts in the page namspace show unintended newlines in footer diff
Closed, ResolvedPublic5 Estimate Story Points

Description

When encountering an edit conflict in the ProofreadPage's page namespace there's an erroneously identified change in the page footer.

The old interface didn't identify any such change for a similar edit:

I assume that this comes from the serialization of the texts that are feed into the conflict. Note, that in the core conflict solving UI this is also visible by looking at the contents of the textareas that show your (below) and the other (above) edit.

See also investigation in T165134: Investigate changes needed for Page namespace on Wikisource

Details

Related Gerrit Patches:
mediawiki/extensions/ProofreadPage : masterRevert "Rtrim input text"
mediawiki/extensions/ProofreadPage : masterRtrim input text

Event Timeline

Xover added a subscriber: Xover.Fri, Feb 14, 6:23 AM

Could this be related to T241889?

If "something" is generating extraneous trailing newlines during editing, but the parser eats them on save, you would get symptoms like this.

Could this be related to T241889?
If "something" is generating extraneous trailing newlines during editing, but the parser eats them on save, you would get symptoms like this.

Hmmm sounds like it. If the wikitext for the diff/preview is retrieved the same as for the conflict resolution then these issues might have the same cause.

WMDE-Fisch set the point value for this task to 5.
WMDE-Fisch moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2020-02-04 board.

Ok one thing I found out already: The reason why there are no newline differences in the diff of the core edit conflict solving screen is, that wikidiff2 is used for the diff there and it trims trailing newlines. If I use the PHP diff engine there we also see the "artificial" newlines in the code.

So the text that is used for the conflict does in any case contain these newlines the your version but no newlines in the stored version. ( as also could already be seen in the textareas of the core screen )

Change 572220 had a related patch set uploaded (by WMDE-Fisch; owner: WMDE-Fisch):
[mediawiki/extensions/ProofreadPage@master] Rtrim input text

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

Change 572220 merged by jenkins-bot:
[mediawiki/extensions/ProofreadPage@master] Rtrim input text

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

Change 572286 had a related patch set uploaded (by Tpt; owner: Tpt):
[mediawiki/extensions/ProofreadPage@master] Revert "Rtrim input text"

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

Tpt added subscribers: awight, Tpt.EditedFri, Feb 14, 5:42 PM

@WMDE-Fisch @awight a significant set of users in Wikisource introduces lines jump at the beginning of the Page: pages body to introduce or not the new paragraph identation. This behavior has been broken for a small amount of time in the past and a significant amount of editors complained. Maybe removing line jumps is the good way to go, but this needs some thought in order to at least get an idea of the number of affected pages.

I just submitted a revert commit, please +2 it.

Xover added a comment.Fri, Feb 14, 6:50 PM

@Tpt rtrim() removes trailing whitespace only, so this shouldn't affect leading newlines in the body of the page, should it?

Tpt added a comment.Fri, Feb 14, 7:20 PM

@Xover. Indeed, I should have read the code more carefully. The existing code seems to trim the line jumps at the end of the header and the line jumps at the end of the footer does not seem to be considered by the parser. So the change should not break anything hopefully. Sorry for the noise.

Change 572286 abandoned by Tpt:
Revert "Rtrim input text"

Reason:
I got confused by trim vs rtrim.

The change that got merged should not create the problem I feared.

For more background, please see https://phabricator.wikimedia.org/T245154#5885428

(sorry for the small description in the commit message and the noise)

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

thiemowmde closed this task as Resolved.Tue, Feb 18, 12:46 PM
thiemowmde moved this task from Demo to Done on the WMDE-QWERTY-Sprint-2020-02-04 board.