Page MenuHomePhabricator

MergeHistory breaks pages with content models that don't allow for redirects
Open, NormalPublic

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 updated the task description. (Show Details)
Bugreporter raised the priority of this task from to Needs Triage.
Bugreporter added a subscriber: Bugreporter.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 21 2015, 9:30 AM
jayvdb added a subscriber: jayvdb.Mar 21 2015, 9:36 AM

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->mTargetID,
        '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.

Anomie set Security to None.
Aklapper triaged this task as Normal 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