Deleted diffs result in blank page (HTTP 200 OK) when one revision is suppressed
Closed, ResolvedPublic
Actions

Assigned To

None

Authored By

	BethNaught
	Nov 13 2016, 8:58 PM

Description

If one revision of a deleted page is non-suppressed and the prior revision is suppressed, the diff link returns an empty page.

Example: log in to en.wp with an admin but not oversighter account. Visit https://en.wikipedia.org/wiki/Special:Undelete/User_talk:94.196.253.228 . Follow the diff link for 19:47 13 November 2016 (directly: https://en.wikipedia.org/w/index.php?title=Special:Undelete&target=User+talk%3A94.196.253.228&timestamp=20161113194759&diff=prev ).

The result is a blank page. Specifically, a 200 OK status code is received with no content.

This isn't high priority, but it would be better if a usual MediaWiki page were displayed with a relevant error message such as MediaWiki:Rev-suppressed-no-diff .

Related Objects

Mentioned Here: T213621: Offer a way to go to the previous or next non-deleted edit in an edit history of a page

Event Timeline

BethNaught created this task.Nov 13 2016, 8:58 PM

Restricted Application added subscribers: JEumerus, Aklapper. · View Herald TranscriptNov 13 2016, 8:58 PM

Aklapper renamed this task from Deleted diffs malfunction when one revision is suppressed to Deleted diffs result in blank page (HTTP 200 OK) when one revision is suppressed.Nov 17 2016, 7:35 PM

@Legoktm ran the request (he has a local admin flag, I prefer not to mess with that kind of thing to test bugs) while I tailed hhvm.log. This showed up:
Dec 4 03:26:06 mw1214: #012Catchable fatal error: Argument 1 passed to DifferenceEngine::generateContentDiffBody() must implement interface Content, null given in /srv/mediawiki/php-1.29.0-wmf.4/includes/diff/DifferenceEngine.php on line 853
Obviously this should have triggered much better handling than an empty page, we should split that to a separate bug though.

The relevant caller is this from SpecialUndelete::showDiff:

$formattedDiff = $diffEngine->generateContentDiffBody(
    $previousRev->getContent( Revision::FOR_THIS_USER, $this->getUser() ),
    $currentRev->getContent( Revision::FOR_THIS_USER, $this->getUser() )
);

DifferenceEngine has code to handle this sort of thing, but it wants revision IDs rather than revision objects. And has a bunch of subclasses.

1997kB moved this task from To triage to Special:Contributions / Special:DeletedContributions on the MediaWiki-Special-pages board.Dec 24 2019, 2:03 PM

1997kB moved this task from Special:Contributions / Special:DeletedContributions to To triage on the MediaWiki-Special-pages board.Dec 24 2019, 2:52 PM

Ammarpad added a project: TestMe.Mar 4 2020, 9:11 AM

The caller has been changed for unrelated reasons and is now getting the complete diff and headers from DifferenceEngine, so this is no longer reproducible as DE already have way to handle that null case. Almost similar to T213621, the classes override DE and then fail to handle edge cases which were already handled well by DE

Deleted diffs result in blank page (HTTP 200 OK) when one revision is suppressedClosed, ResolvedPublicActions

Description

Related Objects

Event Timeline

Deleted diffs result in blank page (HTTP 200 OK) when one revision is suppressed
Closed, ResolvedPublic
Actions