Page MenuHomePhabricator

MergeHistory breaks pages with content models that don't allow for redirects
Closed, ResolvedPublic

Description

Reproduce:

  1. Create Module:Test1 and Module:Test2
  2. Merge Module:Test1 to Module:Test2 using Special:MergeHistory
  3. Now open Module:Test1, It shows:

The revision #0 of the page named "Module:Test1" does not exist.

This is usually caused by following an outdated history link to a page that has been deleted. Details can be found in the deletion log.

If other extensions are also installed, this can cause more strange error (T72827: [1.24wmf20] Fatal error: Call to a member function getProperty() on a non-object, after completely history merging a page), but merging module is also broken even if no other extensions are installed.

Event Timeline

Bugreporter raised the priority of this task from to Needs Triage.
Bugreporter updated the task description. (Show Details)
Bugreporter added a project: Scribunto.
Bugreporter subscribed.

This has nothing to do with Scribunto beyond that Scribunto's Module pages can be used to reproduce the bug. The problem is that Special:MergeHistory doesn't even try to handle the case where the content handler for a page doesn't support redirects.

Quoth SpecialMergeHistory.php:

if ( $redirectContent ) {
    $redirectPage = WikiPage::factory( $_targetTitle );
    $redirectRevision = new Revision( array(
        'title' => $_targetTitle,
        'page' => $this->m_targetID,
        'comment' => $comment,
        'content' => $redirectContent ) );
    $redirectRevision->insertOn( $dbw );
    $redirectPage->updateRevisionOn( $dbw, $redirectRevision );

    # Now, we record the link from the redirect to the new title.
    # It should have no other outgoing links...
    $dbw->delete( 'pagelinks', array( 'pl_from' => $this->mDestID ), __METHOD__ );
    $dbw->insert( 'pagelinks',
        array(
            'pl_from' => $this->mDestID,
            'pl_from_namespace' => $destTitle->getNamespace(),
            'pl_namespace' => $destTitle->getNamespace(),
            'pl_title' => $destTitle->getDBkey() ),
        __METHOD__
    );
} else {
    // would be nice to show a warning if we couldn't create a redirect
}

That "else" case is the problem. Not only is it not showing a warning or anything, it's also leaving the page in a broken state.

Aklapper triaged this task as Medium priority.Mar 23 2015, 4:46 PM
Krenair renamed this task from Merging module using MergeHistory causes non-exist version to MergeHistory breaks pages with content models that don't allow for redirects.Mar 26 2015, 3:44 AM

Change 630580 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Improve handling of content models that do not supports redirect.

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

@Ammarpad Something like this

"If you merged two pages in a [[<tvar|namespace>mw:Special:MyLanguage/Help:Namespaces</>|namespace]] where pages can't redirect this used to break the merge history. This has now been fixed."

I can add it to Tech News when we know which week it'll hit the wikis.

@Johan , Yes basically. But it's not the merge history that breaks itself, it's only the source page. Also the fix now is to record the page as deleted (instead of leaving it in corrupted state), I think that's important to mention

Something like this...

If you merged all revisions of a page into another in a [[<tvar|namespace>mw:Special:MyLanguage/Help:Namespaces</>|namespace]] where pages can't redirect this used to break the merged page. This has now been fixed. The page will be recorded as deleted since there's no more remaining revisions on it"

The patch is not yet merged though, so it's not ready to announce,

Change 630580 merged by jenkins-bot:
[mediawiki/core@master] Improve handling of content models that do not support redirect.

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

  NODES
Note 1
Project 7