Page MenuHomePhabricator

Multiple-issues templates in page sections don't render in modal
Closed, ResolvedPublic3 Story Points

Description

In the new page-issues treatment, multiple-issues templates that are placed in page sections don't show up in the page-issues modal.

This is visible on readers-web-master on page like Stone_Cold_Steve_Austin. Other pages with this template can be found on English wikipedia using this insource search.

This does not happen on pages that have a multiple-issues template in the lead section of the article, such as Organic_compound

Developer notes

For the sake of others trying to understand what the issue is here, I'll try to convey it to the best of my understanding. Unfortunately words are hard, so I made pictures to help.

On the page Samsung, there are two sections with multiple-issue templates, "Operations" and "Acquisitions and attempted acquisitions". The DOM structure between the two is virtually identical.

Acquisitions and attempted acquisitionsOperations

And yet, Acquisitions appears in the modal, but Operations doesn't!

The reason for this is that when we look foramboxes within sections, we split the sections into fake subsections when there is more than one heading, and when we look through that fake subsection, we only select the top-level ambox, whereas when we look through real sections, or sections that do not have subsections, we select all nested amboxes.

When we parse these amboxes further along, we run into a condition that checks if there is only one ambox in the selected set of nodes. If there is, then we proceed, but if there's "not exactly one" (i.e. more than one), then we stop, and the issued don't get added to the dataset that renders them in the modal.

To illustrate:

Acquisitions section

Works because the section doesn't have subsections, and all nested amboxes are selected.


Operations section

Doesn't work because only the top level ambox is selected, which has nested amboxes inside.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 3 2018, 11:41 AM
ovasileva triaged this task as High priority.Sep 3 2018, 2:05 PM
Jdrewniak updated the task description. (Show Details)Sep 3 2018, 2:16 PM
Jdrewniak updated the task description. (Show Details)Sep 3 2018, 9:06 PM

Change 457688 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Fix multiple-issues template in page sections

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

Change 457897 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Equalizing jQuery selectors in Page.findChildInSectionLead

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

ovasileva set the point value for this task to 3.Sep 4 2018, 4:16 PM

during estimation, we decided on a 3 for the time being and decided to also check in next Thursday to see if there is increased complexity revealed

Jdrewniak updated the task description. (Show Details)Sep 5 2018, 10:28 AM

Change 457688 abandoned by Jdrewniak:
Fix multiple-issues template in page sections

Reason:
With a different approach to the following patch:

https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/ /457897/

This change is not necessary.

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

Change 457897 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Change Page.findChildInSectionLead to return nested elements

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

@Jdlrobson, @Tbayer - unsure of how this made its way into signoff...

The bug is fixed. QA will happen as part of T191532.

Jdlrobson closed this task as Resolved.Sep 10 2018, 7:59 PM
Jdlrobson added a subscriber: Niedzielski.

I ran through this with @Tbayer @Jdrewniak and @Niedzielski and this appears to be implemented as expected
I've added some exploratory QA steps to T191532 to cover this scenario further.