Page MenuHomePhabricator

Reply inserted before content at end of line
Closed, ResolvedPublicBUG REPORT

Description

Reply tool inserted a reply in the middle of a line, coincidentally violating explicit instructions that all new comments go below that line.

Expected behavior: Comments are inserted after linebreaks (of course treating EndOfPage as a linebreak).

Testing instructions

  1. Visit a talk page where someone has added a comment instructing others to place new comments below a specified line. E.g. https://en.wikipedia.beta.wmflabs.org/wiki/User_talk:Ppelberg-test#Testing_T268407
  2. Click the [ reply ] link that appears after the following comment: Please add new comments below this notice. Thanks, signed, Rosguill talk 22:16, 10 November 2020 (UTC)
  3. Notice the Reply Tool opens beneath the line mentioned in "Step 2."
  4. Write a comment in the Reply Tool (shouldn't matter what mode)
  5. Publish the comment you drafted in "Step 4."

Actual

  1. ❗️Notice the comment you published in "Step 5." is posted above the line mentioned in "Step 2."

Expected

  1. ✅Notice the comment you published in "Step 5." is posted below the line mentioned in "Step 2."

Done

  • "Expected" behavior is implemented

Event Timeline

DannyS712 changed the subtype of this task from "Task" to "Bug Report".Nov 22 2020, 3:35 PM

@Alsee, can you confirm this is the diff [i] that inspired you to file this task? I'm wanting to be sure we are thinking about the same issue.


i. https://en.wikipedia.org/?diff=988115742&diffmode=source

@ppelberg Yes, that's the one. I fixed the link in the task description.

@ppelberg Yes, that's the one. I fixed the link in the task description.

Great – thank you.

This has been accidentally fixed in rEDTOd0ae6c4e445e: Skip end marker "forward" until a block tag is reached.


The reply tool is able to recognize various "frames" around comments (we initially intended this for barnstar/wikilove messages, but it also recognizes the <div class="xfd_relist" …>…</div> structure), and it will put any replies outside that frame (T250126).

The above did not work in this case, because the wikitext comment <!-- from Template:Relist --> was not considered a part of the discussion comment, and so the frame did not exactly match the comment's extent.

It works now with that patch (I tested locally). Note that the bug would only occur when posting a comment, and not in the "preview" when writing it, because the old PHP parser doesn't output HTML comments for wikitext comments, unlike Parsoid.

The reply is added after the second wikitext comment <!-- Please add new comments below this line --> as a result of the earlier work in T257651.

We should add a test case for this, since it's a rather complicated combination.

Oh, great. By the way, I found the additional context you provided in T268407#6657187 to be helpful; thank you for writing this up, @matmarex.

A couple of follow up comments inline below...


It works now with that patch (I tested locally). Note that the bug would only occur when posting a comment, and not in the "preview" when writing it, because the old PHP parser doesn't output HTML comments for wikitext comments, unlike Parsoid.

  • @Alsee: are the steps listed in the task description's ===Testing instructions section correct?

We should add a test case for this, since it's a rather complicated combination.

  • @matmarex: Do you have a ticket written for this already? If so, can you please mark T269059 as a duplicate of the one you already have?

@ppelberg I spent a while working out what the code is doing, and I'm pretty sure it fails the task. Quote: Never insert a reply in the middle of a line. It appears to be searching for tags and then inserting the reply, disregarding the fact that it may still be in the middle of a wikitext line when it finds that tag.

We sometimes attach a sock-alert or other content immediately after a signature, deliberately keeping it attached on that line. Those alerts are often hand-written, and may contain tags. Inserting a reply at a tag in the middle of a wikitext line will move the sock-identification or other warning onto the reply (and therefore attaching the warning to the wrong person).

Esanders renamed this task from Never insert a reply in the middle of a line to Reply inserted before comment at end of line.Dec 7 2020, 7:50 PM
Esanders renamed this task from Reply inserted before comment at end of line to Reply inserted before content at end of line.

Not quite, the tool distinguishes between inline and block tags (and also comment nodes and meta tags). Trailing inline content will be considered within the reply boundary, and so replies will be inserted after it.

Not quite, the tool distinguishes between inline and block tags (and also comment nodes and meta tags). Trailing inline content will be considered within the reply boundary, and so replies will be inserted after it.

I wasn't sure I was following full background of the code, but thank you for confirming my understanding was correct. Some trailing content is handled correctly but it gets the boundary wrong if it hits a block tag before the end of the wikitext line.

@matmarex please see new test case (revision permalink). The theoretical story there is that I struck an abusive comment, and tacked a bright yellow warning after the signature. It includes a <p> tag. Boom, broken. The reply link gets inserted in the middle of the wikitext line. In that test case the reply tool fails completely. I didn't search to confirm whether it still inserts comments in the middle in other cases. That test case is sufficient to confirm the bug in the code.

Expected behavior: The replies and (reply-links) are not inserted in the middle of a wikitext line. The signature identifies the final wikitext line of the comment. The boundary extends at least to the end of that line, and possibly extends further if the end-of-line appears in the middle of a template or table or something.

@Alsee Thanks for the example, they are always helpful. If you've ever seen similar markup in "real" discussions, can you share the links? I've seen many struck or modified comments, but in my experience such marks are always within one line, or followed by the signature of the person adding the mark (or both).

Based on my experience with talk pages, I'm going to stick with the approach using HTML inline/block nodes, rather than on wikitext line breaks. I haven't seen any real examples where the HTML approach would cause issues (theoretical examples are appreciated but still theoretical), but I have seen real examples where the wikitext approach would cause issues, like this on pt.wp and this on cs.wp.

The HTML-based approach for identifying where comments begin and end also allows us to solve T250126 reasonably (replying to barnstars, wikilove, and other templated comments), without insanely many special cases for tags, tables, templates, and "somethings".

(As a side note, in both of the example edits I linked, the preceding comment doesn't have a line break at the end in wikitext because a prior edit on the talk page was made with the visual editor – you can blame that tool for this, and we don't want people to use VE on talk pages either, but fact is that they do and the new tool should handle this case.)

Merry Christmas. I'll come back to this task after the holidays to finish adding the tests (and fix some issues related to white-space characters, which I've found while adding tests and which luckily don't affect the en.wp "relist" template), then close it.

Change 654293 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Fix detecting decorative comment frames with whitespace

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

Change 654293 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Fix detecting decorative comment frames with whitespace

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

ppelberg claimed this task.