Page MenuHomePhabricator

ToC is not shown during Preview when using the standard WikiEditor
Closed, ResolvedPublic

Description

In the current vector-2022, the ToC does not appear during Preview, if I'm using the standard wikitext editor. (i.e. not using the beta-feature "New wikitext mode").
This makes it impossible for me to understand what results my edit will have, until after I Publish it.

User-story:

  • As an editor who is trying to improve the 'structure' of a page, by adding/editing/refactoring/sorting of the ==Headers== and sub-headers.
  • I want to be able to Preview my changes without saving the page.

E.g. If I add the wrong quantity of === on either side of a heading, I can normally detect this during Preview.

Event Timeline

Jdlrobson added a subscriber: Jdlrobson.

Related conversation in T307251. Presumably the solution there will also be applicable here.

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

[mediawiki/core@master] EditPage: Set sections for new skin TOC when previewing

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

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

[mediawiki/core@master] Fire new 'wikipage.tableOfContents' hook to update TOC on live preview

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

matmarex added a subscriber: matmarex.

This fixes both the full-page preview and the live preview, both didn't show the table of contents, although the issues were different.

Change 802677 merged by jenkins-bot:

[mediawiki/core@master] EditPage: Set sections for new skin TOC when previewing

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

Change 802679 merged by jenkins-bot:

[mediawiki/core@master] Fire new 'wikipage.tableOfContents' hook to update TOC on live preview

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

(original talk) Two editors of the Italian Wikipedia are experiencing the same, no TOC in Preview mode. They are reporting this as a bug. Getting an idea of the layout before saving the change is described as relevant (similar to T294950#789365 by @Quiddity).

Current state (since July) is that the TOC works correctly if you're using old wikitext editor with live preview disabled. Other scenarios don't work correctly (there's a list in T294950#8196609). I think we should close this task, and continue there.

ovasileva raised the priority of this task from Medium to High.

We're going to do some analysis to understand what happened here.

I figured out why it stopped working.

I couldn't reproduce locally first, and I couldn't reproduce on a fresh Patchdemo wiki either, which made me think that could be related to the customized localisation messages shown above the edit form. I knew that some of them are added using OutputPage::addParserOutput, which could potentially override the TOC (since the article content is added in the same way).

I was able to reproduce at first by editing an old revision, which adds one such message. (I found later that it doesn't work on en.wp due to the existence of https://en.wikipedia.org/wiki/MediaWiki:Editpage-head-copy-warn, which is another such message. It works on wikis that don't have it, e.g. pl.wp.)

Once I had a reproduction, I bisected, and found that it is caused by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/869326, which is a change that I was writing right when I left my previous comment on this task, about the feature working correctly :')

As for a fix, I think it would be reasonable to just not override the TOC if the new TOC being added is empty. In the longer term we can invent some better strategy for merging TOCs from multiple outputs (I think @cscott has been thinking about these things recently).

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

[mediawiki/core@master] OutputPage: Do not override TOC data if the new one is empty

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

Jdlrobson updated Other Assignee, added: cscott.
Jdlrobson added a subscriber: bwang.

Huh. In theory you should only have a toc set if you are the "main" parse for the page (in Parsoid terms, the "top level parse"). I'd be interested in digging into how we're ending up with more than one top level parser for the same page.

Following up on T307256#8817705, what seems to be going on is this:

  • OutputPage has various parse* methods for interface messages which ultimately call parseInternal.
  • OutputPage::parseInternal calls parse() with the 'clearState' arg set to true.
  • This causes the parser to instantiate a new ParserOutput object.
  • For some reason that I don't yet understand, this ParserOutput object end up with a non-null TOCData object which then causes the metadata merge problems which Bartosz's patch is working around for now.

So, we need to understand why an interface message parse results in a non-null TOCData object.

Change 914020 merged by jenkins-bot:

[mediawiki/core@master] OutputPage: Do not override TOC data if the new one is empty

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

EAkinloose edited projects, added Verified; removed Editing QA.