I have tried and failed to reproduce this issue. The best we can do for now is ask people to report and look for patterns.

richardg_uk wrote:

Another recent example of loss of all text outside the section being edited, as reported by [[User:Ryan_Vesey]] at [[WP:VPT#Edit_conflicts_with_other_sections.2C_new_issue]]:

http://en.wikipedia.org/w/index.php?title=Talk:Jesus&diff=prev&oldid=519620328

here's a temporary fix that should resolve the problem:

https://gerrit.wikimedia.org/r/#/c/29872/

the merge implementation in TextContentHandler is a buggy, and causing issues I think with wfMerge3()

the temporary fix uses the pre-contenthandler version of the mergeChangesInto EditPage function where it attempts to resolve edit conflicts. This appears to work and fix the problem.

The better solution, of course, is to fix the issue in the content handler but I want a quick fix now.

sumanah wrote:

Both Gerrit change # 29872 and https://gerrit.wikimedia.org/r/#/c/29875/ (the backport to 1.21wmf2) have been merged. But MatmaRex commented in Gerrit on the backport:

"I belive that this causes the following fatal when an edit conflict would happen:

PHP fatal error in /usr/local/apache/common-local/php-1.21wmf2/includes/content/ContentHandler.php line 69:
Argument 1 passed to ContentHandler::getContentText() must implement interface Content, string given, called in /usr/local/apache/common-local/php-1.21wmf2/includes/EditPage.php on line 1479 and defined"

I511b7866 (by Aude) is supposed to fix the fatal.

https://gerrit.wikimedia.org/r/#/c/29881/

sorry about that!

Appears fixed on pl.wiki. Thanks.

sumanah wrote:

https://gerrit.wikimedia.org/r/#/c/29887/ is the backport to 1.21wmf2 by the way.

How i was able to reproduce the bug, with such article text:

intro

Section 1

He was born in 1980.
He likes ice cream.

Section 2

blah blah blah...

2. User A clicks edit (such as section 1)

3. User B clicks edit (e.g. section 1)

4. User B change section 1 to and saves.

He was born in 1990.
He likes ices cream.

5. User A changes section 1 to:

He was born in 1980.
He likes cookies.

6. User A hits save.

It should auto merge that the entire page into:

intro

Section 1

He was born in 1990.
He likes cookies.

Section 2

blah blah blah.

With the bug, it instead automerged the section and saved just the section of the page, erasing the rest of the page, so users would see:

Section 1

He was born in 1990.
He likes cookies.

The issue appears to be TextContent object being passed to the wfMerge3 function in TextContentHandler. It also appears that edit conflicts were happening much more often, such as the mergeChangesIntoContent function in EditPage returning false more often when it should auto merge successfully.

I just wanted the quickest fix that would work, since this was a live bug on Wikipedia, but sure now that we understand the problem, it can be fixed correctly (forward-compatible) without too much difficulty.

Some observations:

• Looking at textContentHandler::merge3, I don't see any way how a TextContent object could be passed to wfMerge.
• Looking at WikitextContentHandlerTest, there is testMerge3, a test case for checking the auto merge functionality. This test is routinely called and passes.

So the problem has to lie somewhere else, probably in the way EditPage handles auto-merging of conflicts of *sections*. I'll investigate some more.

it seems to matter whether User A or User B hits save first.

With User B, I get the blanking.

With User A saving first, I get edit conflict.

This is consistent with reports from Wikipedians that they were getting edit conflicts more often, along with reports of blanking content.

I just got this problem as an edit conflict on en:
Using Firefox 11.0 if that helps.

Wrote test cases for EditPage, including conflict resolution: I8f23c653

I have the issue isolated now, should be able to fix it until tomorrow.

Proper fix posted as I0b1db6d7

ping - could anybody review? Would be great to get this in for wmf3.

Hi Daniel, could you explain your fix a little bit (possibly including a diff between the version before aude's change and your new version)? It's pretty difficult to piece this together just from gerrit, but I think I figured it out. If I'm reading things right, you removed this:

$contentObj = $content;

...and this line:

$this->textbox1 = ContentHandler::getContentText( $contentObj );

...was changed to:

$this->textbox1 = ContentHandler::getContentText( $content );

Is that indeed what changed? If so, why does what you did fix this?

Created attachment 11257
Diff between 0f8a08acb4 and I0b1db6d7 on includes/EditPage.php

Uploading a diff between 0f8a08acb4 (the change before Chad's revert I4c2055be) and I0b1db6d7 (the latest patchset) on includes/EditPage.php.

(Generated using git diff 0f8a08a rMWc9d972a2cc84 -- includes\EditPage.php)

Attached:

(In reply to comment #16)

...and this line:

$this->textbox1 = ContentHandler::getContentText( $contentObj );

...was changed to:

$this->textbox1 = ContentHandler::getContentText( $content );

Is that indeed what changed? If so, why does what you did fix this?

No. Essentially, I replaced this:

if ( $this->mergeChangesIntoContent( $textbox_content ) ) { ... }

By this:

if ( $this->mergeChangesIntoContent( $content ) ) { ... }

The rest is just cleanup.

$textbox_content is holding the content as submitted - for a section edit, just the content of the section. $content holds the full page content before the edit. mergeChangesIntoContent( $content ) merges the new changes into the old content, as it should be. mergeChangesIntoContent( $textbox_content ) merged the new changes into the new section content, which does not make sense, and also loses all content outside that section.

This was a simple oversight - I got confused about which variable holds what and when.

(edit conflict!)

I see what Daniel did and it works, makes sense, etc. Crazy that I didn't figure this exactly out last week. :/

$textbox_content is whatever was in the editing text box. If one clicks the section edit link, then it's just the section. If textbox_content is just a section, replaceSectionContent creates the entire content with the new section content.

for merging conflicts, $content should be used not $textbox_content.

Reopening, I0b1db6d7 (the proper fix) is not merged yet.

Change I0b1db6d7: Status Merged

Closed older resolved bugs as verified.