Page MenuHomePhabricator

No "permanent link" option in sidebar for unreviewed pages in FlaggedRevs
Closed, ResolvedPublicBUG REPORT

Assigned To
Authored By
WindEwriX
Jan 25 2025, 10:56 AM
Referenced Files
F58274372: image.png
Jan 25 2025, 12:56 PM
F58274167: image.png
Jan 25 2025, 12:11 PM
F58274029: image.png
Jan 25 2025, 11:36 AM
F58274023: image.png
Jan 25 2025, 11:36 AM

Description

Steps to replicate the issue (include links if applicable):

What happens?:

  • If it is reviewed now or even has been ever reviewed there are these items:
    • Служебные страницы (Special pages)
    • Постоянная ссылка (Permanent link)
    • Сведения о странице (Page information)
  • But if none of the page versions have ever been reviewed there is no "Постоянная ссылка" (Permanent link) item
  • Although if you use History tab and open any oldid of the page (like 142900024) Permanent link would be displayed in the sidebar

What should have happened instead?:

Software version (on Special:Version page; skip for WMF-hosted wikis like Wikipedia): FlaggedRevs v 27b44e3, MW-1.44

Other information (browser name/version, screenshots, etc.): tested on Vivaldi, Chrome and Firefox, Windows 10, both logged in and logged out

Event Timeline

To figure out how this works on pages without this bug, I created a page with the following revisions:

image.png (268×1 px, 36 KB)

I then visited the page both logged in and logged out. When logged in, the "permanent link" pointed at the unreviewed revision. When logged out, the "permanent link" pointed at the reviewed revision.

With this in mind, it does seem like that the situation in your bug report (the following screenshot)...

image.png (79×1 px, 14 KB)

...should have a permanent link at least for logged in users. And probably logged out users. The permanent link strategy should probably just be to point at whatever revision is being displayed to that user.

But right now, as you say in your bug report, it has no permanent link for either logged in or logged out users. I can confirm this and I am able to reproduce it.

It's also interesting that the revision is not highlighted yellow. That may be a different bug, or the root cause of this bug.

Note that the CSS class for blue highlighting (reviewed) is flaggedrevs-color-1, and the CSS class for yellow highlighting (unreviewed) is flaggedrevs-pending.

Will give this some more thought...

The lack of highlighting of the revisions on the history page of new pages ended up being a different codepath. Filed T384779: in page history, for pages with no reviewed revisions, doesn't highlight the unreviewed revisions yellow and corresponding patch for that.

Root cause of this one is that, in the skin code, $this->getOutput()->getRevisionId() is returning null for the circumstances mentioned in this ticket (pages with FlaggedRevs turned on and with no reviewed revisions).

image.png (591×1 px, 62 KB)

We need to set the revision ID somewhere.

Interesting, why does FlaggedRevs mess with the output of never-reviewed pages to begin with? That’s the case in which it couldn’t do anything anyway…

I investigated if FlaggedRevs is setting $this->output->setRevisionId( null ) somewhere.

image.png (446×560 px, 25 KB)

I set breakpoints at these two locations. These are in showStableVersion() and showOldReviewedVersion() respectively. I could only get showStableVersion() to hit my breakpoint, and this was only for reviewed revisions that were also the top revision. And I didn't see any setting of null there.

So I am not sure what code in FlaggedRevs is overwriting the output revision ID to null.

I changed OutputPage::setRevisionId() to throw an exception if its parameter is null, and then opened an unreviewed page. It didn’t throw. So probably the problem is not that something sets the revision ID to null, but that nothing sets it to non-null.

My assumption was that MediaWiki always sets this in core (this is what powers permalinks on wikis without FlaggedRevs installed, for example), and that an extension would have to override it to null. I could be wrong though, or maybe something besides setRevisionId can change the revision ID sent to the output.

Now I added wfDeprecated( __METHOD__ ); to OutputPage::setRevisionId(), I got:

On a talk page:

Deprecated: Use of MediaWiki\Output\OutputPage::setRevisionId is deprecated. [Called from Article::doOutputFromParserCache in /var/www/html/w/includes/page/Article.php at line 916] in /var/www/html/w/includes/debug/MWDebug.php on line 386

On a reviewed page, and on the stable version of a page with pending changes:

Deprecated: Use of MediaWiki\Output\OutputPage::setRevisionId is deprecated. [Called from FlaggablePageView::showStableVersion in /var/www/html/w/extensions/FlaggedRevs/includes/frontend/FlaggablePageView.php at line 658] in /var/www/html/w/includes/debug/MWDebug.php on line 386

On the pending version of a page with pending changes:

Deprecated: Use of MediaWiki\Output\OutputPage::setRevisionId is deprecated. [Called from Article::generateContentOutput in /var/www/html/w/includes/page/Article.php at line 777] in /var/www/html/w/includes/debug/MWDebug.php on line 386

But I got nothing on an unreviewed article. So it looks like

  • core never sets the revision ID for flaggable pages (which makes sense, as the core setting is in a method called doOutputFromParserCache, and FlaggedRevs plays a lot with the parser cache/parser output), and
  • FlaggedRevs sets it for pages with at least one reviewed version, but it neither sets it nor does it defer setting it (along with parser cache handling) to core on unreviewed pages.

(My core checkout is the latest, but my FlaggedRevs is outdated by months, and I have some local changes because of which it’s not easy to update it. So the line numbers for FlaggedRevs may not be the same, but I hope the bug is the same.)

Update: if a page has not been reviewed, I mark current version as reviewed, and Permanent Link option appears. But then, when I mark this version as unreviewed again, Permanent Link option dissapears again, like here

Change #1114959 had a related patch set uploaded (by Novem Linguae; author: Novem Linguae):

[mediawiki/extensions/FlaggedRevs@master] FlaggablePageView: set the output revision ID for unreviewed versions

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

Change #1114959 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] FlaggablePageView: set the output revision ID for unreviewed versions

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

Novem_Linguae claimed this task.
Novem_Linguae removed a project: Patch-For-Review.