Page MenuHomePhabricator

new section automatic summary parsing does not match core/normal edits
Closed, ResolvedPublicBUG REPORT

Description

Steps to reproduce:

  • Create a new section using the new section interface of discussion tools, with the subject line Bug testing [[Main Page]] (with reply tool)
  • Save

Expected result: automatic summary uses /* Bug testing Main Page (with reply tool) */ new section
Actual result: automatic summary uses /* Bug testing [[Main Page]] (with reply tool) */ new section
(i.e. the brackets should not be included in the automatic summary from the section heading). The normal editing automatic summary strips these from the section title, as does the reply tool for existing section headings

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 changed the subtype of this task from "Task" to "Bug Report".

@DannyS712, I agree that it would be nice if the behaviors matched. Which behavior do you think is the better approach? (What I personal want is: whatever gets saved in the /* edit summary section */, clicking on that in the page history will take me to the right section on the page.)

Workaround:

  1. Click the "Advanced" label under the main input field;
  2. fix the summary manually.

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

[mediawiki/extensions/DiscussionTools@master] Use MediaWiki's new section edit summary if the user didn't modify it

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

I want to provide some commentary with this patch, since there are interesting tradeoffs here.

 

Why this is hard to solve properly
Okay, it's not really hard, but it would be a maintenance burden.

There are two obvious ways to fix this bug:

  1. Reimplement MediaWiki's way of generating the automatic summary in our code

    MediaWiki removes internal links, external links, italics & bold markup, and HTML tags from the section title. Links are easy, but the rest is surprisingly complex – we'd need to rewrite Parser::stripSectionName, Parser::doQuotes and StringUtils::delimiterReplaceCallback (about 300 lines of gnarly PHP) in JavaScript. Also, none of these methods are covered by unit tests, which we'd probably want to write first to ensure we get this right.

    If we do this, we're basically taking on a commitment to maintain the new version, and to ensure that the PHP and JS versions never drift apart.

     
  2. Make an API call to MediaWiki to generate the automatic summary for us

    First of all, there isn't an API that does this, so we'd have to add this feature somewhere, maybe in action=parse. If we do this, we're also making a commitment to maintain this forever (although it would probably be a fairly tiny piece of code).

    Then we have to make the new topic tool fetch the generated summary from this API whenever the user changes the section title. This isn't instant, so we need to make changes to the interface to account for that (e.g. what if the user opens the drawer with the summary textfield before we've fetched the summary? what if they edit the section title, and then the summary while we're waiting for the API response? what what if they try to save before we know what the summary to use?). It's all doable, but it adds complexity, and makes the tool less reliable on poor internet connections.

     

None of this is novel, but it would take a few days to design/implement/test, and more importantly we'd have to maintain it forever.

 

What I'm doing instead

We could very simply avoid all of the complexity if we just didn't let the user change the edit summary. Then we could have MediaWiki generate it as part of saving the page, same as in the wikitext editor's interface for adding a new section. But, although I personally wish that we'd omit this functionality from both tools, it seems that we really want to include it…

However, I realized that if the user never interacts with the summary textfield, we can actually do everything as if it wasn't editable. This would hopefully avoid most cases of incorrect automatic summaries, and the users who want to use a different summary can probably fix it by hand. (Maybe we can add a preview to help them in doing it: T274434.)

I think this approach lets us solve 99% of the problem with 1% of the effort.

Change 722628 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Use MediaWiki's new section edit summary if the user didn't modify it

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

I have checked that this works as expected and the text between the double square brackets is converted to a link as shown in the attached image.

Screenshot 2021-09-28 at 16.54.10.png (272×1 px, 58 KB)

ppelberg added a subscriber: ppelberg.

I think this approach lets us solve 99% of the problem with 1% of the effort.

This sounds like a wise tradeoff to have made, @matmarex. I value how clearly and thoroughly you shared the thinking that led you to move forward with the approach that you did.