Page MenuHomePhabricator

Category template separated from comment on
Closed, ResolvedPublic


Seen quite a few diffs like this:

Looks like the reply tool probably shouldn't be separating these templates from their comments.

Test instructions

  • Write a top level comment, followed by a category, or template that includes a category, e.g. Comment. ~~~~ [[Category:Test]]
  • Reply to that comment with the reply button

The category moves to the end of the page:

The category doesn't move and the reply goes beneath the original comment.

Event Timeline

Esanders created this task.Nov 30 2020, 8:23 PM
Restricted Application added subscribers: Cosine02, Aklapper. · View Herald TranscriptNov 30 2020, 8:23 PM
cscott added a subscriber: cscott.EditedDec 1 2020, 5:24 PM

Foo {{category template}} becomes <p>Foo</p><p><meta....></p> but in a comment :Foo {{category template}} puts the <meta> tag inside the list item as expected?

Apparently this is a very specific bug, where a category template is added *after a signature* in the *first comment in a thread* (the one not inside a list item), and in that case the category gets moved down.

Foo ~~~~ [[Category:Book]]
: Bar ~~~~ [[Category:Book]]


<p>Foo ~~~~ </p>
<link rel="mw:PageProp/Category" href="./Category:Book">
    <dd>Bar ~~~~
        <link rel="mw:PageProp/Category" href="./Category:Book">

Note the category gets hoisted out of the <p> but not the <dd>. I think we saw something similar to this before. It would be nice if Parsoid was consistent and kept the category in the paragraph. The category is in the paragraph if there is text after it.

cscott added a comment.Dec 1 2020, 6:11 PM

In Parsoid this is WTUtils::isRenderingTransparentNode(), which seems to include:

  • HTML comments
  • "SOL-transparent links": <link> tags with rel attributes matching /(?:^|\s)mw:PageProp\/(?:Category|redirect|Language)(?=$|\s)/
  • <meta> tags which don't have `typeof="mw:StartTag" or "mw:EndTag" (these are orphaned literal HTML tags; they might be stripped before Parsoid emits its final HTML)
  • Fallback ID spans: <span typeof="mw:FallbackId"> (found inside headings, you can probably ignore these)

Skipping HTML comments, <meta>, and <link> tags should be good enough, I think.

For context:

Change 645576 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/DiscussionTools@master] Handle category links like comments (rendering-transparent nodes)

Esanders assigned this task to matmarex.Dec 7 2020, 12:25 PM
Esanders moved this task from Upcoming to Doing on the Editing-team (FY2020-21 Kanban Board) board.
Esanders added a project: Editing QA.
Esanders updated the task description. (Show Details)Dec 8 2020, 9:44 PM

Change 645576 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Handle category links like comments (rendering-transparent nodes)

A particularly complex case we should add to the integration tests:

A closing div, followed by a comment, empty tracking template and category

Change 647219 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/DiscussionTools@master] Skip over empty inline nodes (e.g. tracking templates)

Change 647219 merged by jenkins-bot:
[mediawiki/extensions/DiscussionTools@master] Skip over empty inline templates (e.g. tracking templates)

JTannerWMF moved this task from Inbox to High Priority on the Editing QA board.Dec 16 2020, 6:37 PM

Working as expected it seems. On the diff view, the reply is now being placed right beneath the first comment that includes the category template.

The diff view:

The page:

Ryasmeen edited projects, added Verified; removed Editing QA.Dec 16 2020, 8:41 PM
ppelberg closed this task as Resolved.Dec 24 2020, 2:33 AM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptDec 24 2020, 2:33 AM