Page MenuHomePhabricator

section edits sometimes wrongly merged in edit conflicts
Closed, ResolvedPublic

Description

Author: elian

Description:
Should be a known issue, but I couldn't find a bug report:

If you insert a new section (say number 5) and someone is editing section 12
below and saves it, no edit conflict is given, section 12 is just inserted
another time (see http://test.wikipedia.org/wiki/Edit_conflicts for examples and
testing). There are other reports that sometimes sections are deleted
unvoluntarily in similar cases.

Workaround: fix page later manually, make problem known in the editing FAQ.


Version: unspecified
Severity: major

Details

Reference
bz56

Revisions and Commits

Event Timeline

bzimport raised the priority of this task from to High.Nov 21 2014, 6:46 PM
bzimport set Reference to bz56.
bzimport added a subscriber: Unknown Object (MLST).

wikibug.5.starfury wrote:

This problem is a major one, if it isn't noticed immediatly! (Or do some users just don't care?)

e.g. look at
http://de.wikipedia.org/w/wiki.phtml?title=August_2004&diff=2342294&oldid=2342256
there wasn't noticed from August 30 11:01 till 20:30, while several changes where made.
http://de.wikipedia.org/w/wiki.phtml?title=August_2004&action=history
(I noticed it only because it happened again while I was editing the page!)

rowan.collins wrote:

I suspect this is related to or the same as bug 275 (which is older than it
seems; I just migrated it from SourceForge). However, given that that bug
diagnoses it as a problem of conflict-with-self, and this suggests conflict
between users, I won't mark it as a duplicate just yet, as it may be that there
are two bugs here, making each other worse.

wmahan_04 wrote:

Patch to fix bug

Here is a patch that attempts to fix the problem by merging section edits into
the revision at the time the edit form was loaded, rather than always using the
current revision.

Attached:

wmahan_04 wrote:

Please note that the problem pointed out by "Addicted" might be a different bug;
as far as I could tell that problem duplicated the entire contents of the page,
rather than one section. My patch aims to fix a problem wherein sections are
duplicated or dropped. Similarly, I'm not sure that bug 275 is the same bug. But
I am fairly certain it will fix the problem described by the bug reporter, elian.

The problem my patch addresses can occur either in a conflict between two
different users, or in the case of conflicting submissions by the same user. It
can be difficult to predict what the the edit conflict merging code might do
after the article text gets mangled. Thus I can't rule out the possibility that
these problems are related.

wmahan_04 wrote:

*** Bug 69 has been marked as a duplicate of this bug. ***

wmahan_04 wrote:

Patch against MediaWiki 1.3.3

I am uploading a patch against MediaWiki 1.3.3, since this seems to be a
serious bug that might be worth fixing in the stable branch.

attachment wtm-bug56-1.3.patch ignored as obsolete

Wil, can you provide a sequence of edits which reproduces the bug and which is
fixed by the patched code? These section things are tricky and I just want to
make dead sure it's nailed. :)

wmahan_04 wrote:

Sure. Please look at http://test.wikipedia.org/wiki/Bug56 . I initially created
a page with sections 1-4, filled with some dummy text. Then I initiated two
section edits at the same time, for section 2 and section 4 (i.e. I loaded both
section edit pages before submitting anything else).

On the section 2 edit page, I added a new section above section 2 and submitted.
As you can see from the second revision in the page history, everything works
fine to this point. But then in the section 4 edit form, I added some text to
the section (although the specific nature of this edit isn't important for
reproducing the bug; any changes to the text are sufficient).

Note that when I submit the form for section 4, section 3 gets dropped entirely,
while the article now contains copies of section 4 both before and after my
edit. This is because when I submit section 4, the updated text is inserted into
the article based on the section numbering of the most recent revision, rather
than the section numbering at the time I loaded the edit form.

This bug can occur because of a self conflict, as this test case demonstrated,
or due to a conflict between two different users. Let me know if you have any
other questions.

wmahan_04 wrote:

In case my previous explanations weren't clear, here's some pseudocode
describing what my patch does. When a user submits edits to section number
"N" of an article, the current code (without my patch applied) roughly
does the following:

// start pseudcode

get_current_text_of_article() //might have changed since we started editing

nothing below here is changed by my patch
code to update section
break_the_text_up_into_sections_and_number_them()
delete_section_N_and_all_subsections_of_it()
insert_the_new_submitted_edits_at_section_N()

// merge if necessary
if (the_article_has_been_modified_since_the_user_started_editing) {

if the_last_edit_was_by_the_same_logged_in_user() {
   // self-conflict; just overwrite the old revision
   put_the_text_into_the_db() 
}
else {
  //uses the "diff3" program
  merge_our_text_with_the_current_revision_in_the_db()
  if (the_merge_succeeded) {
    put_the_merge_result_into_the_db() 
  }
  else {
    give_the_user_the_edit_conflict_page()
  }
}

}
else {

put_the_text_into_the_db()

}

// end pseudcode

I simplified some things, but that should be the general idea. The
problem with the above is that the section updating code does not
take into account whether the article was modified since the user
began editing. So the "delete_section_N" part could end up deleting
the wrong section, which the merging code can't fix because in
effect, it's the same as if the section were removed by hand. The
deletion/insertion of the wrong section is merged into the final
result.

My patch only changes one thing: instead of
get_current_text_of_article()
it effectively does
get_text_of_article_at_the_time_we_started_editing()

I think it is clear that even when someone else has submitted changes
while we were editing, those changes will still get merged in
because the most current (most recent) revision is always used for
the merge.

Hope I didn't just add to the confusion. :)

wmahan_04 wrote:

I commited my patch to HEAD. If no problems turn up I will see about applying it
to REL1_3.

wmahan_04 wrote:

Patch against REL1_3

Updated patch against REL1_3 that fixes a warning. If possible, I would like to
have this applied to 1.3 to see if it helps with bug 275.

Attached:

mediazilla wrote:

This may be related to bug 649.

wmahan_04 wrote:

*** Bug 649 has been marked as a duplicate of this bug. ***

1.4 imminent, closing as FIXED.

Diffusion added a commit: Unknown Object (Diffusion Commit).Mar 4 2015, 8:18 AM