Page MenuHomePhabricator

Log instances of infoboxes being wrapped in containers
Closed, ResolvedPublic1 Story Points

Description

 Context

During T149389, we discovered that there exist pages whose infoboxes are wrapped in one or more containers. Since this was a warning being triggered in production, we fixed the bug by ignoring these infoboxes.

Before we can provide the same treatment for all pages, we need to know how many pages have infoboxes wrapped in containers and which templates produce them.

 AC

  • If there's an infobox in the lead section and it's inside a container, then log the title and revision of the page.
  • This behaviour is feature flagged.

 Hints

The cheapest way to detect if the infobox element is inside a container is to test whether its parent is the section container, i.e.

$infobox = $infoboxAndParagraphs->item( 0 );

if ( $leadSectionBody !== $infobox->parentNode ) {
  // The game is afoot...
}

 Resources

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Before we can provide the same treatment for all pages, we need to know how many pages have infoboxes wrapped in containers and which templates produce them.

My concern is that we don't know the scope of the problem: if there's a variety of templates producing infoboxes in one or more containers or if there's just a handful of pages with odd wikitext. For the former, I'd recommend we come up with a general approach and change MobileFormatter but for the latter, we might get away with fixing the wikitext.

@Jdlrobson, @bmansurov: rEMFR4bce0adef858: Only consider immediate descendant infoboxes while moving first paragraph disabled infobox relocation when the infobox was wrapped in a container. This task is about helping us to understand why/when infoboxes are wrapped in containers and allow us to get a better handle on relocating 'em too.

@ovasileva: Is there a timeline for T150325: Move first paragraph before infobox on stable?

@phuedx - I was hoping next sprint (Jan 4 - 18), but given how much we have left over, I think it will probably be the sprint after.

ovasileva raised the priority of this task from Normal to High.Jan 4 2017, 6:19 PM

Hi, I might be able to help, if the task is just understanding where these are containers are coming from? Please could you point towards the existing example(s) that you found? (I couldn't see anything in the older task, but might've missed it)

At a guess, you might be meaning the modular infoboxes (a fancy template hack, intended primarily [IIUC] to reduce fragmentation/duplication of template parameters), as used in places such as:

All of the above templates/techniques, are duplicated to some degree, at other language wikis.
There's further information on the related issues in:

(Apologies if I'm misunderstanding the point here! :-) )

Please could you point towards the existing example(s) that you found? (I couldn't see anything in the older task, but might've missed it)

Hey, @Quiddity. Thanks for the brain dump! We'll be sure to take a look at some uses of the templates to see if we can get to the bottom of this. As for examples that we've found: we haven't! This task is about adding some instrumentation so that we can collect examples.

Jdlrobson updated the task description. (Show Details)Apr 5 2017, 5:47 PM
Jdlrobson set the point value for this task to 1.
Jdlrobson moved this task from Needs Analysis to To Do on the Reading-Web-Sprint-95 board.
pmiazga claimed this task.Apr 5 2017, 7:05 PM
pmiazga moved this task from To Do to Doing on the Reading-Web-Sprint-95 board.
phuedx added a comment.Apr 6 2017, 8:26 AM

I'm glad to see that we're doing this. Do bear in mind that triggered notices, warnings, and errors appear to be logged with wiki and title information now (woo!) so this might be as simple as triggering a notice. OTOH logging the issue to a custom channel is much cleaner.

Change 346927 had a related patch set uploaded (by Pmiazga):
[mediawiki/extensions/MobileFrontend@master] Log infobxes being wrapped in containers

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

Jdlrobson moved this task from Needs QA to Ready for Signoff on the Reading-Web-Sprint-95 board.
Jdlrobson removed a project: Patch-For-Review.

To verify this we may need to wait, or to purposely create a case on the beta cluster that we know will trigger this warning.

Change 346927 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Log infoboxes being wrapped in containers

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

Jdlrobson reassigned this task from pmiazga to phuedx.Apr 10 2017, 5:09 PM
Jdlrobson added a subscriber: pmiazga.

I've created a[n] edit which should give a warning
https://en.m.wikipedia.beta.wmflabs.org/wiki/Hello_Piotr_I%27ve_been_expecting_you
However I'm not seeing it on https://logstash-beta.wmflabs.org ...

Same. The page that you created certainly triggers the code path on my dev wiki – I see the following entry in the -debug.log log file:

mediawiki-wiki-debug.log:2017-04-11 14:45:06 wiki MobileFrontend DEBUG: Found infobox wrapped with container on T149884 (rev:47)

But I can't find it anywhere on logstash-beta.

Change 347619 had a related patch set uploaded (by Phuedx):
[mediawiki/extensions/MobileFrontend@master] formatter: Change log channel of infobox message

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

^ From the commit message:

By default, the minimum log level for Logstash is INFO. The minimum log level for the "mobile" channel, however, is DEBUG.

I've changed the channel for the message to "mobile" as it both seems more appropriate and it has the right minimum log level. To be clear, this could've also been fixed by raising the log level of the message to INFO.

Change 347619 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] formatter: Change log channel of infobox message

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

Still not able to replicate this on logstash.. am I using the right query?

Change 347802 had a related patch set uploaded (by Phuedx):
[mediawiki/extensions/MobileFrontend@master] formatter: Change log level of infobox message

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

Still not able to replicate this on logstash.. am I using the right query?

Yes.

T149884#3172114 and rEMFR9da62ce9bb01: formatter: Change log channel of infobox message were only partially correct. The minimum log level for the Logstash really is INFO.

Change 347802 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] formatter: Increase log level of infobox message

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

SWAT sounds like a good idea, especially, considering that we need this data sooner rather than later.

phuedx reopened this task as Open.Apr 12 2017, 12:29 PM

Per the above.

phuedx removed phuedx as the assignee of this task.Apr 12 2017, 12:29 PM
phuedx moved this task from Ready for Signoff to To Do on the Reading-Web-Sprint-95 board.

Change 347886 had a related patch set uploaded (by Phuedx):
[mediawiki/extensions/MobileFrontend@wmf/1.29.0-wmf.20] formatter: Change log channel of infobox message

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

Change 347886 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@wmf/1.29.0-wmf.20] formatter: Change log channel of infobox message

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

Change 347896 had a related patch set uploaded (by Thcipriani; owner: Phuedx):
[mediawiki/extensions/MobileFrontend@wmf/1.29.0-wmf.20] formatter: Increase log level of infobox message

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

Mentioned in SAL (#wikimedia-operations) [2017-04-12T18:38:24Z] <thcipriani@tin> Synchronized php-1.29.0-wmf.20/extensions/MobileFrontend: SWAT: [[gerrit:347886|formatter: Change log channel of infobox message]] T149884 (duration: 00m 46s)

Change 347896 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@wmf/1.29.0-wmf.20] formatter: Increase log level of infobox message

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

Mentioned in SAL (#wikimedia-operations) [2017-04-12T18:45:37Z] <thcipriani@tin> Synchronized php-1.29.0-wmf.20/extensions/MobileFrontend: SWAT: [[gerrit:347802|formatter: Increase log level of infobox message]] T149884 (duration: 00m 46s)

Jdlrobson removed a project: Patch-For-Review.

This is now live in wmf20 and will thus be live on enwiki tomorrow.
Hopefully some data will start appearing in logstash soon

Okay.
I found some.

To take an example ... Eddie Murphy.
אדי מרפי (rev:20475684)

I'm a little worried this might be logging incorrectly and will do so for every edit. Looking at that page, Eddie Murphy is a child of #mf-section-0 which is true of any page with an infobox.

@pmiazga ... can you investigate?

The first patch has silly mistake and it logs all infoboxes

Change 347990 had a related patch set uploaded (by Catrope; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@wmf/1.29.0-wmf.20] Log only infoboxes which are not a direct children of lead section

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

Change 347990 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@wmf/1.29.0-wmf.20] Log only infoboxes which are not a direct children of lead section

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

Mentioned in SAL (#wikimedia-operations) [2017-04-12T23:37:18Z] <catrope@tin> Synchronized php-1.29.0-wmf.20/extensions/MobileFrontend/: Log only infoboxes which are not a direct children of lead section (T149884) (duration: 01m 05s)

Jdlrobson closed this task as Resolved.Apr 12 2017, 11:51 PM
Jdlrobson claimed this task.
Jdlrobson removed a project: Patch-For-Review.

Not a 1 pointer. I've confirmed that
log events after deployment will show up in this query for infoboxes wrapped in containers but not normal infoboxes.

I'm going to consider this resolved. Follow up work is tracked in T162713.

Just in case you were talking about infoboxes-nested-in-infoboxes (as I described earlier), I made a demo at https://en.wikipedia.beta.wmflabs.org/w/index.php?title=Asdasfasgadgasda&action=edit (assuming those 3 templates are roughly equivalent to the originals found on Enwiki).

@Quiddity - yes, we will log all wrapped infoboxes (even if they are inside different infobox).