Page MenuHomePhabricator

Handle signatures within transcluded templates or "subpages"
Closed, ResolvedPublic

Description

I can't think of any. But looking around I did notice this page [iii] that has a welcome template with an embedded comment, which probably should not be treated as such. The edit box shows up but when I type something and click "Reply", nothing happens.

iii. https://nl.wikipedia.org/wiki/Overleg_gebruiker:Uitgeverij_Lipari?dtenable=1

Yes, this is a bug. We talked about handling signatures within comments/transclusions, but haven't implemented it yet.

In this case the only thing we can do is to not allow replying to this comment. This actually behaves the same to the situation where a discussion is closed by wrapping it in a template – I can't find an example in Dutch, but here's one in English: https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(proposals)/Archive_164#Proposal:_Removing_Technical_Move_Requests_Process. The comment is still on this page, but because it's inside a template, we're not able to insert a reply to it.

(We also want to handle the situation where an entire discussion thread is transcluded from another page, which is a common workflow in deletion discussions, apparently also on Dutch Wikipedia: https://nl.wikipedia.org/wiki/Wikipedia:Te_beoordelen_pagina%27s?dtenable=1&dtdebug=1#Toegevoegd_03/02;_af_te_handelen_vanaf_17/02. In this case, the expected behavior would be to reply on the subpage, e.g. https://nl.wikipedia.org/wiki/Wikipedia:Te_beoordelen_pagina%27s/Toegevoegd_20200203.)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 20 2020, 12:14 AM

This task sounds big and scary but we've been thinking about this from the start, when deciding how posting replies should work, so it should be easier than it looks like.

@matmarex, before the replying workflow is able to support posting comments within transcluded templates or "subpages," can you think of a straightforward way of preventing reply links from appearing within them?

Once we have modified the parser to detect subpages we could hide these links, but:

  1. Having done that we will be about 50% of the way to supporting subpage editing anyway and
  2. We can only detect subpages once the Parsoid HTML has finished loading in the background, so either the reply links would start enabled then disable some time later, or we would have to delay all reply links from showing until the Parsoid HTML has finished loading.

tl;dr is I don't think there is a "quick fix" here, but subpage support should be prioritised

ppelberg added a comment.EditedMar 4 2020, 2:11 AM

Got it – thank you for laying this out, Ed.

Below are the notes from the conversation Ed and I had on 25-February.

Included in these notes are: the three cases [we are aware of] where contributors are unable to respond to comments within templates or on subpages.

Next steps

  • Confirm which of the below cases the approach we have in mind (is this documented somewhere?) will and will not support.
  • For Case #1 what – if any – options do we have to prevent reply links from appearing?

Cases

Case #1: A single comment is generated by a templates

  • Example: {{Welcome3. | ~~~~}} as @Bdijkstra identified here.
  • Support: the approach we have in mind for resolving this ticket WILL NOT enable people to reply to comments generated by a template because the signature is inside of the template.

Case #2: An entire discussion page that's transcluded from another page

Case #3: An entire discussion is wrapped in a template

  • Example: {{Discussion top}} / {{Discussion bottom}}
  • Support: the approach we have in mind for resolving this ticket WILL ENABLE us to prevent reply links from appearing "on" comments that are wrapped in a template.
matmarex claimed this task.Mar 4 2020, 4:17 PM
JTannerWMF added a subscriber: JTannerWMF.

Before working on this ticket, @Esanders will write the questions and assumptions discussed in planning with Peter.

Change 576949 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] parser: Detect comments transcluded from another page

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

Just to recap (this should answer your questions @ppelberg):

  • Current behavior:
    • "Reply" links appear for transcluded comments, reply widget appears when clicking them, and posting your reply seems to succeed, but actually nothing is saved.
  • Expected behavior: any of the options below would be better:
    • Trying to post a reply actually saves it (we should be able to do this for case #2, not implemented yet)
    • Trying to post the reply displays a helpful error message (we can do this for case #1/#3, the patch above already implements it)
    • No "Reply" links appear for transcluded comments (we might not be able to do this, because we need to load the Parsoid HTML first before we can determine that a comment is transcluded)

This patch is a step but not a complete solution yet. It only implements detecting whether we're in case #1/#3 or in case #2 (or neither), and displays an appropriate error message for either case. It doesn't implement replying on the subpage, we should work on that next (I'm not doing it currently, but could pick it up).

(Also, case #1 and #3 are basically the same, it doesn't matter whether a single comment or multiple comments/threads are transcluded)

ppelberg added a comment.EditedMar 5 2020, 2:08 AM

Thank you, @matmarex.

A couple questions before writing ticket(s) for the cases r/576949 does not support:

  • 1. Error message timing: When does the "This comment can't be replied to using this tool. Please try using the wikitext editor instead." error message appear? After a contributor attempts to publish (read: click "Reply" button) the comment they've just written? If so, what is the reason for us not showing that message before they go through the effort of writing a comment?
    • I appreciate the behavior https://gerrit.wikimedia.org/r/576949 introduces is an improvement over the current experience (some feedback is better than no feedback). Tho, I'm curious what – if anything – standing in the way of us communicating this information to people sooner.
  • 2. Case #2 error message: Is this the error message that will appear in Case #2 instances: "This comment can't be replied to here (yet), because it is loaded from another page. Please go to [[$1]] to reply to it."? If so, where does clicking [[$1]] lead someone to?
  • 3. Parsoid HTML: Does, "...we might not be able to do this... [i] mean it's technically not feasible or implementing it borders on complexity that leads you to question whether the effort is worthwhile?
    • Reason I ask: I left this morning's meeting thinking this was possible.
  • 4. Replying on a subpage: is this [ii] what you understand support for "...replying on the subpage..." to mean?

i.

  • No "Reply" links appear for transcluded comments (we might not be able to do this, because we need to load the Parsoid HTML first before we can determine that a comment is transcluded)

ii.

@ppelberg I created a wiki running this patch and made up some content for testing, you can try it out here: http://patchdemo.wmflabs.org/wikis/d32e83e3946a480f5171cb8c70d7a2e6/w/index.php/Talk:Transcluded

On that page, you can reply in the first section, but can't reply in the second or third.


  1. It only appears when trying to post a reply. It should appear earlier, but we will need to refactor the order in which we load things to implement that.
  2. It leads to the subpage (you can try it out in that demo now). Maybe we'll just delete this, it might not be needed if we implement posting the reply directly to the subpage.
  3. It's technically easy, but with our currently architecture, it would affect the page load performance (basically we have to load the entire page twice, for complicated reasons). For this reason I don't think it's a good idea, so that's why I wrote "might not be able to do this". To do it without worsening the performance, we'd have to move a bunch of processing to the server (rewrite the code from JS to PHP), which is probably a good idea in the long term, but it will take more time to do.
  4. Yes

Change 576949 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] parser: Detect comments transcluded from another page

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

Change 578387 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] controller: Show error messages immediately when loading fails

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

ppelberg added a comment.EditedMar 11 2020, 4:51 PM

Being able to try out the actual behavior is a big help; thank you for putting this together, @matmarex.

Responses/comments in-line, below.

@ppelberg I created a wiki running this patch and made up some content for testing, you can try it out here: http://patchdemo.wmflabs.org/wikis/d32e83e3946a480f5171cb8c70d7a2e6/w/index.php/Talk:Transcluded

This looks great. One tweak:

  • For cases #1 and #3 (comments/discussions that are wrapped in templates), are you able to change the error message from "This comment can't be replied to using this tool. Please try using the wikitext editor instead." to "This comment can not be replied to using this tool because the comment exists inside of a template. To reply to this comment, please try using the wikitext editor instead: LINK TO WIKITEXT EDITOR."
    • Thinking: people should have clear expectations about what comments/conversations the reply tool does and does not support. I think providing specific error messages will help people form these expectations.

  1. It only appears when trying to post a reply. It should appear earlier, but we will need to refactor the order in which we load things to implement that.

Understood. This approach is a good start.

  1. It leads to the subpage (you can try it out in that demo now). Maybe we'll just delete this, it might not be needed if we implement posting the reply directly to the subpage.
  • This is looking good. RE error messaging, are you able to change the error message from "This comment can't be replied to here (yet), because it is loaded from another page. Please go to SUBPAGE LINK to reply to it." to "This comment can not be replied to using this tool (yet), because the comment is loaded from another page. To reply to this comment, please go to SUBPAGE LINK."
    • Thinking: see above about clear expectations.
  1. It's technically easy, but with our currently architecture, it would affect the page load performance (basically we have to load the entire page twice, for complicated reasons). For this reason I don't think it's a good idea, so that's why I wrote "might not be able to do this". To do it without worsening the performance, we'd have to move a bunch of processing to the server (rewrite the code from JS to PHP), which is probably a good idea in the long term, but it will take more time to do.

Understood. Let's hold off for now.

  1. Yes

Great.


Next steps

  • @ppelberg to file task for Case #2: An entire discussion page that's transcluded from another page [i]
  • eng. to implement error message copy edits (see above)

i.

Case #2: An entire discussion page that's transcluded from another page

This looks great. One tweak:

  • For cases #1 and #3 (comments/discussions that are wrapped in templates), are you able to change the error message from "This comment can't be replied to using this tool. Please try using the wikitext editor instead." to "This comment can not be replied to using this tool because the comment exists inside of a template. To reply to this comment, please try using the wikitext editor instead: LINK TO WIKITEXT EDITOR."
    • Thinking: people should have clear expectations about what comments/conversations the reply tool does and does not support. I think providing specific error messages will help people form these expectations.

I'm not sure about these changes:

"because the comment exists inside of a template"

I avoided explaining why the comment is uneditable because it's a bit more difficult to explain why. There are some weird corner cases and I preferred an incomplete message rather than an occasionally incorrect message.

There are the two cases you're probably thinking of:

  • {{wrapper|* comment ~~~~}} (indeed inside of a template)
  • {{top}}* comment ~~~~{{bottom}} (arguably inside of a template? I'd say it's between two templates)

But occasionally a comment would be uneditable when it's before or after a template:

* comment 1 ~~~~
{{weird template}}
* comment 2 ~~~~

If "weird template" generates a list item, comment 2 will be uneditable. A similar issue in VE is described in T185093 (that task has some technical explanation why this happens).

In this case comment 2 is not "inside of a template". You could say it's "affected by a template" or "a part of a transclusion", but I don't think either of those are a useful explanation for someone just trying to comment on a talk page.

Although I guess this is unlikely to happen often in discussions, so maybe I'm worrying unnecessarily…

"LINK TO WIKITEXT EDITOR"

I avoided including a link because it's always confusing to me when some interface has multiple buttons that do the same thing. I always get the urge to compare them and make sure they really work exactly the same. Is it just me?

  • This is looking good. RE error messaging, are you able to change the error message from "This comment can't be replied to here (yet), because it is loaded from another page. Please go to SUBPAGE LINK to reply to it." to "This comment can not be replied to using this tool (yet), because the comment is loaded from another page. To reply to this comment, please go to SUBPAGE LINK."
    • Thinking: see above about clear expectations.

Thanks, "this tool" instead of "here" sounds better.

I'm not sure how I feel about repeating the word "comment" so many times.

  • @ppelberg to file task for Case #2: An entire discussion page that's transcluded from another page [i]

I filed T247535: Save the reply directly to the source page when a discussion comment/thread is transcluded from another page, feel free to fill in the details.

I also filed T247533: Show error messages immediately when loading fails since I want to document some thoughts about how we should display those errors – I'll move my proposed patch there.

The only thing left on this task is to finalize the wording.

I'm not sure about these changes:

"because the comment exists inside of a template"

I avoided explaining why the comment is uneditable because it's a bit more difficult to explain why. There are some weird corner cases and I preferred an incomplete message rather than an occasionally incorrect message.

Understood – thank you for showing why it's difficult for us to explain precisely why someone is unable to respond to the comment they are responding to.

I filed this task to refine they message copy to more clearly to communicate to people why they are unable to respond this particular "category" of comments: T248607.

"LINK TO WIKITEXT EDITOR"

I avoided including a link because it's always confusing to me when some interface has multiple buttons that do the same thing. I always get the urge to compare them and make sure they really work exactly the same. Is it just me?

Let's talk about this in: T248607

I'm not sure how I feel about repeating the word "comment" so many times.

Agreed. Let's also talk about this in: T248607

  • @ppelberg to file task for Case #2: An entire discussion page that's transcluded from another page [i]

I filed T247535: Save the reply directly to the source page when a discussion comment/thread is transcluded from another page, feel free to fill in the details.

Thank you.

I also filed T247533: Show error messages immediately when loading fails since I want to document some thoughts about how we should display those errors – I'll move my proposed patch there.

The only thing left on this task is to finalize the wording.

Great. As mentioned above, let's do that in: T248607

ppelberg closed this task as Resolved.Mar 26 2020, 6:12 PM