Page MenuHomePhabricator

Lead paragraph transform should not log if the page is already mobile friendly
Closed, ResolvedPublic3 Estimated Story Points

Description

We log every time we are unable to rearrange the infobox and lead paragraph so the lead paragraph comes first. We only log for English Wikipedia but would like to reduce the amount of logging so we can track any major changes to major templates that our code depends on. It seems we log events for pages where the lead paragraph is already above the infobox which adds unnecessary noise.

When visiting https://en.wikipedia.org/wiki/P38_mitogen-activated_protein_kinases, even though the infoboxes on this page are wrapped, they do not appear at the top of the page. Despite this, we log that the page is not mobile friendly "message:Found infobox wrapped with container on P38 mitogen-activated protein kinases (rev:840755052) "

We should only log pages where the infobox appears above the first paragraph and cannot be moved. I believe, if this was addressed, we would see no errors logged (or near zero) and possibly reconsider our daily chore in the chores list.

Developer notes

https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/includes/transforms/MoveLeadParagraphTransform.php#L96

In the 2nd if statement the method hasNoNonEmptyPrecedingParagraphs( $xPath, $infobox ) is not taken into account in logging. Adding a test and adding a check should be straightforward.

QA notes

This should not make any difference, but there is a slight risk that this may impact the lead paragraph algorithm.
Please verify nothing has broken by looking at a variety of articles on http://reading-web-staging.wmflabs.org/ and confirming the lead paragraph shows above the infobox on mobile.

Sign off steps

A developer should look at logstash and should notice a decrease in number of events logged complaining about unmoveable paragraphs.

Event Timeline

Jdlrobson created this task.Jun 8 2018, 6:45 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 8 2018, 6:45 PM
Jdlrobson updated the task description. (Show Details)Jun 8 2018, 7:33 PM
Jdlrobson triaged this task as Medium priority.Jun 12 2018, 4:12 PM
Jdlrobson updated the task description. (Show Details)Jun 12 2018, 4:17 PM
Jdlrobson set the point value for this task to 3.
Vvjjkkii renamed this task from Lead paragraph transform should not log if the page is already mobile friendly to kdbaaaaaaa.Jul 1 2018, 1:05 AM
Vvjjkkii raised the priority of this task from Medium to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed the point value for this task.
Vvjjkkii removed a subscriber: Aklapper.
CommunityTechBot renamed this task from kdbaaaaaaa to Lead paragraph transform should not log if the page is already mobile friendly.Jul 2 2018, 1:13 PM
CommunityTechBot lowered the priority of this task from High to Medium.
CommunityTechBot set the point value for this task to 3.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added a subscriber: Aklapper.

Change 443523 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Lead paragraph transform should not log if the page is already mobile friendly

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

Change 443523 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Lead paragraph transform should not log if the page is already mobile friendly

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

@alexhollender, I don't think there's any design sign off needed for this task. Is that your understanding too? It sounds like a developer will need to QA it.

pmiazga removed alexhollender as the assignee of this task.Jul 17 2018, 12:38 PM
pmiazga removed a project: Patch-For-Review.
pmiazga added a subscriber: alexhollender.

Logs are clean, I'm resolving this task

pmiazga closed this task as Resolved.Jul 17 2018, 12:44 PM