Page MenuHomePhabricator

Support timestamped signatures generated by "unsigned" templates that haven't been substituted
Closed, ResolvedPublic

Description

Not all wikis use subst with unsigned templates, e.g. https://fr.wikipedia.org/wiki/Mod%C3%A8le:Non_sign%C3%A9

It should be possible to reply to these as the rest of the comment is not wrapped in a template.

Event Timeline

I thought this should already work, but of course it doesn't.

This is the responsible part of parser.js:

function getTranscludedFrom( comment ) {
	var node, about, dataMw;

	// If some template is used within the comment (e.g. {{ping|…}} or {{tl|…}}), that *does not* mean
	// the comment is transcluded. We only want to consider comments to be transcluded if the wrapper
	// element (usually <li> or <p>) is marked as part of a transclusion.
	// TODO: This seems to work fine but I'm having a hard time explaining why it is correct...
	node = comment.range.endContainer;

That will handle templates within the comment, but not those in the signature at the end of it.

I remembered an earlier version of that code, which instead had node = nativeRange.commonAncestorContainer;: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/576949/1..5/modules/parser.js#b925

…but IIRC that didn't work correctly for multi-line comments.

Change 599117 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Support replying when timestamp is template-generated

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

Change 599119 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Tests covering fr.wp unsigned comment templates

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

Change 599120 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Proposed fr.wp template tweak

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

Change 599120 abandoned by Bartosz Dziewoński:
Proposed fr.wp template tweak

Reason:
Merged into https://gerrit.wikimedia.org/r/599119

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

Change 599117 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Support replying when timestamp is template-generated

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

Change 599119 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Tests covering fr.wp unsigned comment templates

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

I've been using this page for testing: https://fr.wikipedia.org/wiki/Discussion:Le_Monde#Mort_de_Pierre_Berge

There are two unsigned comments in that section. After this patch goes live, it should be possible to reply to the second one, but not the first one – there are some further problems with multi-line comments.

I think modifier#getFullyCoveredWrapper should actually return an array of sibling nodes, rather than a single node. Currently this doesn't work for multi-line comments (see the first case in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DiscussionTools/+/599120/1/tests/cases/fr-unsigned-parsoid/fr-unsigned-parsoid-transcludedFrom.json)

Change 606000 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] [WIP] Improve detecting template-generated multi-line comments

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

Did you plan to merge the patch before T257646?

Change 606000 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Improve detecting template-generated multi-line comments

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

I've been using this page for testing: https://fr.wikipedia.org/wiki/Discussion:Le_Monde#Mort_de_Pierre_Berge

There are two unsigned comments in that section. After this patch goes live, it should be possible to reply to the second one, but not the first one – there are some further problems with multi-line comments.

I think I've successfully replied to the first unsigned comment in the == Mort de Pierre Berge == section [i]; @matmarex
is that problematic and/or evidence that further work needs to be done?


i. https://fr.wikipedia.org/w/index.php?title=Discussion_utilisateur%3APPelberg_%28WMF%29%2FSandbox&type=revision&diff=174110166&oldid=174110156

I think I've successfully replied to the first unsigned comment in the == Mort de Pierre Berge == section [i]; @matmarex
is that problematic and/or evidence that further work needs to be done?

No, it's supposed to work now.

After the first set of patches on this task, when I wrote that comment, replying to that comment didn't work. But after the latest patch it should work.

I think I've successfully replied to the first unsigned comment in the == Mort de Pierre Berge == section [i]; @matmarex
is that problematic and/or evidence that further work needs to be done?

No, it's supposed to work now.

After the first set of patches on this task, when I wrote that comment, replying to that comment didn't work. But after the latest patch it should work.

Understood. I'm resolving this task considering the evidence posted in T252058#6407874 suggest everything is working as expected.