Page MenuHomePhabricator

DiscussionTools appends emptystate banner on `action=parse`
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):
Testing this on the English Wikipedia. Make any action=parse request on the API with title set to a user talk page and text containing no talk sections, the issue appears. In this case, the following is used (see P32287 for full cURL output):

curl -v --url https://en.wikipedia.org/w/api.php --header "content-type: multipart/form-data" --form action=parse --form prop=text --form "title=User talk:Chlod" --form "text={{copied|from=A|to=B}}" --form format=json --form disablelimitreport=true

What happens?:
DiscussionTools emptystate banner shows up on output. The only way to disable this is by setting disableeditsection to true. Setting preview does not work. Here's the preview (without images, but the images do appear when the HTML is appended and CORS works):

Screenshot_7.png (665×1 px, 88 KB)

What should have happened instead?:
The banner shouldn't be present on parsed HTML. I'm still unsure as to why something that's supposed to be part of the interface shows up on parsed HTML, especially when no specific wikitext triggers the banner. Setting disableeditsection feels like a hack, nothing explicitly triggered the banner so I should never expect it to appear. When attempting to render preview text (as seen above), this ends up polluting the output and causes a disparity with other wikis without DiscussionTools (despite using the same wikitext).

Event Timeline

FYI, in next week's MediaWiki release, preview=1 will suppress this again, and disablelimitreport=1 will no longer suppress this. (This is a side-effect of the changes for T314260, we didn't really think how this would affect API use.)

I'm not sure if the banner should be present on parsed HTML or not in general. I think it makes sense when parsing the current wikitext of the page (e.g. https://en.wikipedia.org/wiki/Special:ApiSandbox#action=parse&format=json&page=Talk%3AS._A._Beach&prop=text). If it was separate from the parsed page content, then at least some editing tools like DT and VE would need to separately take care to display or hide the banner after making an edit.

Do you think that just making preview=1 suppress it is enough (assuming we document it as a supported feature and don't break it again), or should we think about some other options?

The use case being you have just edited a page and the banner should be shown. An editor like VE which updates the page dynamically does so by fetching the new page contents from action=parse, so this needs to return the full rendering of the content area, including this banner.

I'm still unsure as to why something that's supposed to be part of the interface shows up on parsed HTML

This is also true of section edit links

especially when no specific wikitext triggers the banner.

the wikitext lacking any signatures triggers the banner. It may be less intuitive that section edit links, because its the lack of something rather than the presence of something (a header).

Setting disableeditsection feels like a hack

This is the only existing option for disabling page editing UI within the content. It would probably be better if it was called disableeditinterface but I don't think it's a hack.

I would not expect the banner to be in the parsed output since action=parse is meant to parse the content (wikitext) of the page, and the banner is not present in the wikitext.

Do you think that just making preview=1 suppress it is enough (assuming we document it as a supported feature and don't break it again), or should we think about some other options?

I'm alright with just having preview=1 suppress this (and for the documentation, definitely, since this came as a surprise to me); see below for some ideas.

This is also true of section edit links

The same could also be said with the NewPP limit report (albeit it remains a comment) — no specific wikitext triggers it but we do expect it to be there unless explicitly disabled. Those who use the API (like me, in this case) expect it since it is native to the MediaWiki core and it's documented on the action=parse's help page.

But unlike the section edit links and limit report, DiscussionTools (as of now) runs off of an extension. This specific problem didn't appear to me on my testing wiki because the extension wasn't installed there. Testing it on the English Wikipedia surprised me since there was no documentation that said API output would be affected, nor did I do something (or did not do something, such as include a disableeditinterface parameter) that led to the banner showing up. Specifically in P32287, all I needed was how a template would look on a talk page and since I am intentionally passing no sections (and hence don't find the need to enable disableeditsection) and no signatures, I expected just the template output, nothing more. With DT, this is no longer the case and I get more than I asked for (text={{copied|from=A|to=B}}).

An editor like VE which updates the page dynamically does so by fetching the new page contents from action=parse, so this needs to return the full rendering of the content area, including this banner.

Would it be possible for the parser to return hidden metadata (an HTML comment or a blank <span> or possibly an action=parse prop to avoid modifying the parser HTML entirely) that VE would be able to detect so that it can append the banner on the page after loading the parsed content? I'm sure the banner (considering it's only 1,199 bytes long in the output linked above) could fit in client-side JS without that much of a performance hit, but I'm not savvy enough in extending the API to know if that's in the realm of possibility.

append the banner on the page after loading the parsed content

I would rather not do this. It would result in delayed loading, and a repaint of the page as the height changes.

matmarex claimed this task.

preview=1 removes the banner since this week's deployment. (A week later than what I wrote in my earlier comment T314623#8134133, because there was no deployment train that week: https://lists.wikimedia.org/hyperkitty/list/wikitech-l@lists.wikimedia.org/thread/7RBKZ6ICPVGLAX6GFHRIGSRKVMRN424Q/.)

I think we can call this resolved.

It doesn't seem to be documented anywhere. You can find the code affected by this option here: https://codesearch.wmcloud.org/search/?q=getIsPreview. Seems to be a lot of random things in random extensions, and also the {{REVISIONUSER}} magic word.