[go: up one dir, main page]

Page MenuHomePhabricator

"Add topic" / "New section" tab, and the DiscussionTools empty state, sometimes disappears after the page is edited
Closed, ResolvedPublic

Description

"Add topic" / "New section" tab, and consequently also the DiscussionTools empty state message ("Start a discussion about…"), sometimes disappears after the page is edited (even when the edit doesn't add any comments or headings that would disable the empty state).

I can reproduce this on https://en.wikipedia.beta.wmflabs.org/wiki/Talk:Empty_page. No idea why it happens.

Reported by @Xaosflux at https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#c-Xaosflux-20220805200400-Matma_Rex-20220805183700

Event Timeline

Sshot, with missing tab/missing empty state

image.png (517×1 px, 215 KB)

This happens because OutputPage::isRevisionCurrent() can incorrectly return false due to reading stale data from the replica. It also affects some other features not related to our talk page work – notably, when the bug occurs, the "Edit source" tab has an unnecessary &oldid=… parameter.

To fix this, we will need to tell OutputPage whether the revision being shown is "current" according to the rest of the page rendering code, rather than leaving it to find out on its own. (Just reading from the master database wouldn't be correct, since it could cause the same issue in the opposite direction if everything else was reading from a stale replica.)

I am surprised no one has noticed this before. I don't know if it's a new issue, or if no one really pays attention to the tabs disappearing.

How I found this out:

  • I couldn't reproduce the bug locally, which made me think this could be an issue with database replication – specifically, if some parts of the code were reading from a replica that doesn't have the just-saved revision yet
  • I reviewed the code responsible for deciding to show the empty state ((DiscussionTools) HookUtils::shouldShowNewSectionTab()), and the only thing that looked like it could be affected by the above is a call to OutputPage::isRevisionCurrent()
  • I looked for anything else that uses OutputPage::isRevisionCurrent(), to see if it is broken in a similar way. I found that the URL of the "Edit source" tab depends on it to add &oldid=…: Skin::editUrlOptions()
  • And sure enough, when the issue occurs, the "Edit source" tab has an &oldid=… parameter, that actually links to the correct (current) revision, even though it is only added if the revision isn't current:
    image.png (2×3 px, 728 KB)

Change 820845 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] OutputPage: Provide consistent info about whether the revision shown is current

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

Change 820845 merged by jenkins-bot:

[mediawiki/core@master] OutputPage: Provide consistent info about whether the revision shown is current

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