Page MenuHomePhabricator

Hatnote and ambox recognition is poor and essentially only works for enwiki
Closed, ResolvedPublic

Description

As part of T262093, the code in MobileFrontend that moves the first paragraph of an article was updated to work better, and more consistently with the app version. What the new code does is, extract the leading paragraph, look for any hatnotes or amboxes, and pull up the paragraph to after those "higher-priority templates".

While this might be a good idea, the current implementation (r625346) basically only accounts for the content of these templates on enwiki: it looks for elements having "hatnote" or "ambox" as CSS class, except that there's no fixed convention for using these AFAIK.

As a case in point, I took the "Other uses" template on enwp, which uses "hatnote". I looked at the equivalent templates on itwiki, frwiki and dewiki, and none of these are using this class. For ambox, I used the equivalent of {{unreferenced}}. Only enwiki and frwiki use "ambox", whereas itwiki and dewiki don't. I haven't investigated any further, but this looks already worrying enough to me.

This effectively means that the change broke the mobile layout on "non-compliant wikis" (except we can't talk about compliance, if there's no standard to comply with), where the first paragraph is now the very first element of a page, which creates a bad experience for users.

I haven't tried whether the mobile app has the same issue, but either way, it is my opinion that this change should be immediately reverted on the wmf.34 branch, and fixed/reimplemented afterwards with better detection of important templates.

Event Timeline

For the record, taking itwiki as an example and using data from turnilo, in the last week there have been 96m page views from the mobile web version, as opposed to just 2.7m from the mobile app. For this reason (and because "more uniform" doesn't necessarily mean "better"), I believe that this change should be reverted regardless of what the status quo is in the mobile app.

Change 671150 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/MobileFrontend@wmf/1.36.0-wmf.34] Revert "Rewite MoveLeadParagraphTransform based on mobile apps approach"

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

From our discussion on IRC:

The revert patch would have to be reviewed by the mobile team, I have subscribed to this task a few of them that interact on the original change ( https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/625346 ).

Marking this regression as a blocker of the deployment task would raise awareness about it as being important. And surely people got notifications (it is also unbreak now).

So I guess it is all about getting the revert reviewed by the team in charge, then they can deploy it or the train conductor can assist in rolling the patch.

Thank you to have spotted the issue and for all the commits and the cherry pick!

can you point us to some example URLs and/or screenshots where we can see the broken state?

I'm assuming the hatnotes here are what's being referred to but I don't want to make any assumptions.


https://it.m.wikipedia.org/wiki/Regno_Unito

Another temporary solution would be to temporarily remove .nota-disambigua elements from the page until they are made standard in the template. This can be done via a config change by changing wgMFRemovableClasses.

If an urgent fix is needed, I suggest the templates are patched using the instructions on https://www.mediawiki.org/wiki/Recommendations_for_mobile_friendly_articles_on_Wikimedia_wikis#Use_standardized_class_names_in_HTML_markup_for_components_in_templates_across_projects

As explained in T277258, dewiki for example uses class="navigation-not-searchable" on the disambiguation notice. There are absolutely no standard classes for whatever elements of a page across projects (the infobox class is still not standard on dewiki, and I think on itwiki it had never even been used).

can you point us to some example URLs and/or screenshots where we can see the broken state?

You can get plenty of examples by following inclusions of the templates linked above. Not just hatnotes, but also amboxes. See for instance here. The more templates, the bigger the impact, e.g. here.

Another temporary solution would be to temporarily remove .nota-disambigua elements from the page until they are made standard in the template. This can be done via a config change by changing wgMFRemovableClasses.

I don't think this is a viable option, for five reasons:

  • You'd also have to hide all amboxes
  • "Temporary" solutions have a bad habit of becoming permanent
  • That is an important warning for users. Right, some templates are alredy hidden, but I don't think that's a valid reason for hiding more.
  • You'd need to propagate (a localized version of) the same change to all wikis
  • If we can afford a config deployment, I believe we can afford a revert.

If an urgent fix is needed, I suggest the templates are patched using the instructions on https://www.mediawiki.org/wiki/Recommendations_for_mobile_friendly_articles_on_Wikimedia_wikis#Use_standardized_class_names_in_HTML_markup_for_components_in_templates_across_projects

While this might be a good idea for the long term, I again think this is not a good idea, for the following reasons:

  • It would need to happen on all wikis
  • I find the revert a more appropriate urgent fix
  • I'm not very keen on making changes to templates used on hundreds of thousands of articles. I'd do that only if necessary (again, it might be fine for the long term, but not as an urgent fix)

I think the simplest approach is to just "Revert first, fix later". This change disrupted the mobile experience for millions of users. Can't we just revert it and then redo it better?

I think the simplest approach is to just "Revert first, fix later". This change disrupted the mobile experience for millions of users. Can't we just revert it and then redo it better?

I can't get hold of the people involved in that patch to make the call to the revert (European hours). HTML is involved so I'm not sure if reverting would cause any caching problems.
I think a Friday revert is thus looking unlikely until that happens so wanted to make sure all the possible options are known.

Ps. This is a good reminder that we need more wikipedias in group 2. Right now we only have Catalan and Hebrew. This could have been caught yesterday and reverted yesterday.

I think the simplest approach is to just "Revert first, fix later". This change disrupted the mobile experience for millions of users. Can't we just revert it and then redo it better?

I can't get hold of the people involved in that patch to make the call to the revert (European hours).

I guess we can wait for Monday, if that's our only option.

HTML is involved so I'm not sure if reverting would cause any caching problems.

I can't answer this, but I should note that changing all those templates (on all wikis) would likely have a similar impact.

Ps. This is a good reminder that we need more wikipedias in group 2. Right now we only have Catalan and Hebrew. This could have been caught yesterday and reverted yesterday.

+1, and as I wrote above, I can't stress enough how important it is to test this kind of things on something more than just enwp.

For dewiki, I have now added hatnote to some of the most common templates. Luckily I have admin rights, since many of these templates are protected. But this can indeed not be the solution; as long as the recommendations linked above are not followed by a substantial percentage of the projects, such an enwiki-centric change disrupts a huge number of projects. Just saying, I am one of the main forces behind mobile-friendly optimisations on dewiki, but I had never even seen this page with recommendations before. I will be happy to promote them on dewiki, but there would be a lot more to do in other projects. Revert by Monday sounds reasonable for now.

But this can indeed not be the solution;

Requiring specific classes to be added was how the old approach worked as well, and it too was broken in various places because of this. Over time we have managed to encourage wikis to use a standard infobox class, and many use the ambox classes. Enforcing a standard for hatnote (which is defined at https://w.wiki/35ur) would be no different, and probably only requires fixing a small number of templates at each wiki to cover 99% of cases.

The failures do not look that serious, putting small hatnotes under the lead is less serious issue (and has seemingly gone unnoticed on the mobile apps for years) than the failure cases in the previous approach, where an infobox can appear above the lead paragraph. Also any fixes we make will be permanent.

I still think this is the correct approach, so by reverting we are just saying the wikis need more time to fix, but my removing the code we also remove the incentive the fix, and the ability for users to test. If we revert we may be in a similar position a year from now.

The failures do not look that serious, putting small hatnotes under the lead is less serious issue

I think we can all agree that this is not a site-breaking issue or a major showstopper. However, it cannot be dismissed as a minor issue. This change introduced accessibility issues with many articles on many wikis. In my opinion, it's actually impossible to figure out exactly how broken things might be. That would require knowing what kind of templates are getting moved around on all wikis. My analysis above was just 4 wikis, but the list is long. Also, if you ask me, the infobox above the lead is a much less serious issue than templates being moved between paragraphs.

(which has seemingly gone unnoticed on mobile apps for years)

I think this can be easily explained by looking at the metrics posted at T277302#6908661. To expand a bit on that, I checked the pageviews in the past 30 days, excluding enwiki (link). Mobile app requests made up roughly 1.4% of the total page views. If you consider that some people might not even understand that this is an issue; or that they might just be too lazy to report it; then, I don't see anything strange. Again, consistency is not an excuse here, for we simply cannot compare "mobile web" vs "mobile app" in terms of traffic.

and these can be pretty easily fixed permanently by amending the templates.

Right, but see below.

I also think this is still the correct approach, so by reverting we are just saying the wikis need more time to fix, but my removing the code we also remove the incentive the fix, and the ability for users to test.

This is another problem with the approach that was taken here. No notice (or time) was given to anybody to make the necessary changes. I bet that many people have never even heard about those recommendations on mw.org. @XanonymusX said this above, and I'm doing it now for myself. I think a sane approach here would be to:

  • First, post a notice on Tech News, mentioning the upcoming change and inviting people to do whatever might be necessary.
  • Then, after some time, implement the change. At that point, you can indeed say that removing the code might remove the incentive to fix the templates.

Alternatively, if we really care about testability and user-friendliness, this might be moved behind a config flag. You could then progressively turn it on for more and more wikis.

And again, as I mentioned above, changes like this should really be tested on more wikis. This is what would really avoid problems like this one.

But as it stands, all that I see is a change that broke the wikis. Sure, you may say it's not a serious issue, but still, it is an issue. I am failing to understand what's the problem with reverting. It's just the standard practice if something breaks and cannot be fixed quickly enough. In fact, I think this discussion could be avoided altogether if we just revert, send out notices, and then redo after a while. For this reason, unless another acceptable solution is found, I'm going to self-merge my revert on master before the next train.

Then yes, I might want to update the templates where I have the rights to do so (aka itwiki). Others might want to do the same on other wikis. But this doesn't—and shouldn't—matter. The crux here is that the implementation broke stuff, and there wasn't enough notice; as such, a revert is necessary.

One more reason why the change should be reverted: <templatestyles> tags break hatnote/ambox recognition. On itwp, amboxes are using the "ambox" class, and hatnotes are using the "hatnote" class (thanks @Esanders who added it). Yet the layout is broken, since the hatnote templates uses TemplateStyles, which injects a tag at the very beginning of the page. This doesn't seem to be the case on the same set of wikis examined above (en, fr, de).

I remember similar problems on dewiki with the IPA template, which uses templatestyles and had previously broken the moving of the first paragraph. That had been fixed (last year?), can’t remember which task that was.

If I had to choose between having to add the infobox class to all infoboxes on a wiki or to add hatnote to all hatnotes (I’m not sure if people agree to introduce ambox on dewiki), I would probably go for the latter, as the number of potentially affected templates is much smaller (I had recently gone through a list of infoboxes without the class on dewiki, still a huge number) and the infobox class also doesn’t work well with all infoboxes (some nested tables break, and text-align is affected). But indeed, the communities need to be notified about such changes!

One more reason why the change should be reverted: <templatestyles> tags break hatnote/ambox recognition. On itwp, amboxes are using the "ambox" class, and hatnotes are using the "hatnote" class (thanks @Esanders who added it). Yet the layout is broken, since the hatnote templates uses TemplateStyles, which injects a tag at the very beginning of the page. This doesn't seem to be the case on the same set of wikis examined above (en, fr, de).

I filed this issue as T277367 and there is already a fix up for it.

One more reason why the change should be reverted: <templatestyles> tags break hatnote/ambox recognition. On itwp, amboxes are using the "ambox" class, and hatnotes are using the "hatnote" class (thanks @Esanders who added it). Yet the layout is broken, since the hatnote templates uses TemplateStyles, which injects a tag at the very beginning of the page. This doesn't seem to be the case on the same set of wikis examined above (en, fr, de).

I filed this issue as T277367 and there is already a fix up for it.

Oh, I didn't see that. But still, I don't see any reason for rushing a fix. There are still broken templates around. There might be more special cases that we don't know about and would require a similar fix. Let me put it this way: why add band-aids and urge communities to rush a fix, instead of reverting, sending out appropriate notice, testing on some more wikis, and then redoing the change after a while? It doesn't have to be months. One or two weeks would suffice.

Reverting is fairly standard when things break and a quick solution cannot be implemented. Unless someone wants to update every relevant template on all wikis before the next train, I'm not seeing any reason not to follow the standard practice here.

T277381 is also a task that I just noticed. I think that might be a plausible consequence of rush-fixing high-visibility templates on a bunch of big wikis, and one more reason to let the fixes distribute over a longer time frame.

MediaWiki might publish a global Style Guide for such issues.

  • Then projects do need 12 months at least to implement all such features.
  • All class names are to be prefixed by: mw-
  • A global dictionary of selectors exposed to the public and supposed to be utilized should be distributed immediately by MediaWiki design department.
  • A .mw-page-position-top can be applied where desired then.

It is not acceptable to take the local customs of enwiki and use them as unpublished global standard for 1000 WMF wikis. This is a kind of cultural domination and patronizing style which can be observed as well with so-called global templates and global modules. It actually means: Everybody has to do everything exactly as enWP does. MediaWiki is supporting enwiki only, and all others are obliged to follow enwiki community decisions.

  • T132308#3263787 For the record: Software and services that are used on hundreds of wikis are not required to abide by the policies or guidelines of any individual wiki.

dewiki has no problem to equip all their related templates with an mw- class.

  • Our Template:Hinweisbaustein does know |POSITION=oben which actually means position=top and could trigger an mw- class with one edit only.
  • The box design for top position is omitting top and side border lines.

I am looking forward to get informed about the MediaWiki selector dictionary with all these mw- classes.

Yeah, I just experimented with the ambox class and must say that it is absolutely incompatible with related templates on dewiki. Which means that we would have to use hatnote on all of them (not really what the class was designed for) or just leave them in the middle of the text if this change were to become permanent. Apart from potential clashes with local CSS because of poor naming, as pointed out by @PerfektesChaos, the biggest issue is that these classes all have very specific styling attached to them, which has caused me quite some headache with infobox already; now with hatnote there comes a grey text-colour and a tiny font size, which is not in line with standard styling on dewiki.

Do one thing, and do it well.

  • I did suggest a class mw-page-position-top.
  • That does say: This block shall be arranged before introduction section.
  • Nothing more.

Those local ambox and hatnote and infobox are decoration classes. They define how this particular wiki is decorating things. That is not a statement on the exact position where to arrange it in a page.

  • Structural global semantics are one thing.
  • Local decoration is something different.

All of the statements above apply equally to the way this transformation was done before, relying on a selector that matched .infobox, .tright and anything else that might float right at the top of the page. These selectors and .ambox and .hatnote were already used in the code before this patch, so this isn’t a new issue. I do agree it would be better if these classes were prefixed and separated from content styles.

MediaWiki might publish a global Style Guide for such issues.

It does: https://w.wiki/35ur

It is not acceptable to take the local customs of enwiki and use them as unpublished global standard for 1000 WMF wikis.

I agree in principle, but this is the status quo with the infobox classes and others for now, and we may not have the resources to fix it any time soon.

Change 671150 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@wmf/1.36.0-wmf.34] Revert "Rewite MoveLeadParagraphTransform based on mobile apps approach"

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

Mentioned in SAL (#wikimedia-operations) [2021-03-15T12:32:34Z] <urbanecm@deploy1002> Synchronized php-1.36.0-wmf.34/extensions/MobileFrontend/: 41a2aaac8c7b6ee5ec05af6d051d541614eaba30: Revert "Rewite MoveLeadParagraphTransform based on mobile apps approach" (T277302) (duration: 00m 58s)

For ambox, I used the equivalent of {{unreferenced}}. Only enwiki and frwiki use "ambox", whereas itwiki and dewiki don't. I haven't investigated any further, but this looks already worrying enough to me.

The Italian equivalent is https://it.wikipedia.org/wiki/Template:F which does use ambox, the German one has already been fixed. Many smaller wikis already copied their templates from en.wiki, so I think adoption of ambox is already quite high (the same applies to hatnote).

Daimona claimed this task.

For ambox, I used the equivalent of {{unreferenced}}. Only enwiki and frwiki use "ambox", whereas itwiki and dewiki don't. I haven't investigated any further, but this looks already worrying enough to me.

The Italian equivalent is https://it.wikipedia.org/wiki/Template:F which does use ambox, the German one has already been fixed. Many smaller wikis already copied their templates from en.wiki, so I think adoption of ambox is already quite high (the same applies to hatnote).

This might be true, but I'd rather let the communities find out what fix might be needed before going live. I'm calling this task resolved, and will track the follow-ups on the parent.

Please let the appropriate team close the task.

In fact, I think this discussion could be avoided altogether if we just revert, send out notices, and then redo after a while.

FWIW this is why I reverted the change. It'll take time and a little effort to set up a broader testing environment (thanks to great tools like patchdemo). That's all.

@phuedx We currently have a en.wiki checkbox on patchdemo, but that could easily be extended to other wikis.

Jdlrobson lowered the priority of this task from Unbreak Now! to Needs Triage.Mar 15 2021, 5:05 PM

Patch has been reverted so removing UBN.

The UBN is taken care of. Let's continue the conversation in T262093.