Page MenuHomePhabricator

Determine if .navboxes should be stripped from the page content delivered by the Mobile Content Service and Page Content Service
Closed, ResolvedPublic

Description

I noticed that https://en.m.wikipedia.org/api/rest_v1/page/mobile-sections/Barack_Obama/ is returning giant tables which are never shown on mobile. iirc these are stripped on MobileView, greatly reducing transfer size for many larger articles.

On the Obama article the "This article is part of a series about..." table is one such example:
(screenshots are from Desktop - on mobile we're sending these giant tables but there's a min-width media selector which keeps them hidden)

So are the giant tables at the bottom of the Obama article:
Collapsed:

Expanded:

Event Timeline

Mhurd created this task.Sep 5 2017, 10:43 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 5 2017, 10:43 PM

The first one is a table.vertical-navbox. The other screenshots are from a div.navbox. MCS currently removes table.navbox. I could easily add those selectors to the transformation list if that is what is desired, and I'm leaning towards doing so for consistency sake.

@Fjalapeno, any thoughts?

@Mhurd thanks for bring this up!

@bearND We need to do some investigation on this and find out why these tables were removed them in MobileFrontend in the first place.

Going forward, I think we want to try to preserve as much of the page and editor intention as possible, so it may be time to review previous decisions and try to make it possible to display some of this on mobile devices.

@Jdlrobson @phuedx do you have any insights on the stripping of these tables in MobileFrontend?

@Jdlrobson @phuedx is this something we should look to TemplateStyles to fix?

@Jhernandez @phuedx when it comes to Marvin I think we want to try and preserve as much of the original content as possible, even if that means we display the the content differently or extract it as structured data. Do you have any thoughts here on what you would like from the Page Content Service?

Fjalapeno renamed this task from "mobile-sections" is delivering giant tables which are never displayed on mobile (and iirc are stripped out on MobileView) to Determine if large tables should be stripped from the page content delivered by the Mobile Content Service and Page Content Service.Sep 6 2017, 1:19 PM
Fjalapeno claimed this task.
Jdlrobson added a comment.EditedSep 6 2017, 1:36 PM

What does "large tables" mean?

Currently we strip navboxes from the html as a short term solution. I'd expect this to continue to happen in MCS.

What does "large tables" mean?
Currently we strip navboxes from the html as a short term solution. I'd expect this to continue to happen in MCS.

@Jdlrobson I guess that is what I am asking you. @Mhurd gave an example of what is stripped in the description. Is it just navboxes?

Looks like there was some discussion about this last year:
T124168: Show Navbox templates in mobile skins

bearND added a comment.Sep 6 2017, 4:40 PM

The examples mentioned in the description are variation of navboxes.

Mhurd updated the task description. (Show Details)Sep 6 2017, 8:23 PM
Mhurd added a comment.Sep 6 2017, 8:26 PM

As I mentioned in the description, it looks like these are hidden on mobile by a min-width media selector. So the only effect of removing this html which is therefor never shown is the transfer would be smaller and faster :)

In mobile web, neither navboxes nor vertical-navboxes are shown, because of the size they have and their usage of tables, making them unsuitable for mobile viewport sizes.

I don't know if there are any plans to do something with them in mobile web or marvin (ping @Nirzar & @ovasileva).


How does it happen:

Both are hidden via CSS, because there was a time where mobileview didn't strip navboxes because of performance concerns:

https://github.com/wikimedia/mediawiki-skins-MinervaNeue/blob/master/resources/skins.minerva.content.styles/hacks.less#L20-L23

	.vertical-navbox,
	.navbox {
		display: none;
	}

Currently, AFAICT, mobile formatter removes by default in mobileview:

	".toc",
	".navbox",
	".nomobile"

Which is then overriden in the production config to just:

'wgMFRemovableClasses' => [
	'default' => [
		'base' => [ '.navbox', '.nomobile' ],
		'beta' => [ '.navbox', '.nomobile' ],
		'HTML' => [],
	],
],

Navboxes are removed because they are impossible to format and show on a mobile viewport, because of their use of tables and the styles they have.


Re size, from previous research we did, we found that navboxes html could get to be even 20% of the HTML size (Barack Obama).

I believe that's around the time we enabled navbox stripping in the PHP API, and since it didn't pose any performance problems it remained enabled.


I hope this is helpful, let me know if I can help explain something else.

@Jhernandez very helpful, thanks!

For @Nirzar and @ovasileva:

I think we have 3 choices here:

  1. Strip navboxes out of the PCS (like we do now in MobileFormatter/MobileFrontend
  2. Include navboxes, but rely on Template Styles to fix the display
  3. Strip navboxes, but return the content as structured data so that clients can build custom views to display them

I'm guessing that option 1 is the least desirable as far as community concerns.
Option 2 requires lots of adoption of template styles, and opens up the question of what we do with navboxes that haven't been properly styled.
Option 3 is good because it doesn't rely on template styles adoption, but requires clients to implement the views (not unlike what we are doing for reference lists).

I'm leaning towards 3 myself, but also have to investigate if we can truly return navboxes as structured data reliably. And there may be another solution that we haven't thought of yet.

Why are we overcomplicating this? Just remove them like we do everywhere else. We can change this decision later if the Navbox template works on desktop, but that's not happening any time soon. Let's not over-engineer this.

@ovasileva and I talked about this today.

We decided to strip them away for now. we don't have product and design resource to rethink the usefulness of navboxes in a structured and/or responsive way. as we don't know the usecase and we don't know how we might make them work on phones, let's strip them for now.

In the future we would want structured content for navboxes and we can think about nicer and usable interfaces for it and then inform the services.

@Nirzar thanks for the info. That sounds reasonable… knowing that you want to do it in a structured way in the future makes this easy.

@Jdlrobson I appreciate you wanting to just move on because you solved in in MobileFrontend, but I don't want to make unilateral decisions to remove content that editors add to pages and has been served by the MCS for a year without getting background… and also not without getting input from the engineers, PM and designers on a new project that will depend on this service.

We are building a new service that will probably be serving all the HTML for all our clients for the foreseeable future…we don't want to have to make breaking changes in 2 years to support something we never took a few moments to discus on Phabricator early on in the process.

Fjalapeno closed this task as Resolved.Sep 8 2017, 2:35 AM

Closing, made task for actual implementation

Jdlrobson renamed this task from Determine if large tables should be stripped from the page content delivered by the Mobile Content Service and Page Content Service to Determine if .navboxes should be stripped from the page content delivered by the Mobile Content Service and Page Content Service.Sep 8 2017, 12:50 PM

> We are building a new service that will probably be serving all the HTML for all our clients for the foreseeable future…we don't want to have to make breaking changes in 2 years to support something we never took a few moments to discus on Phabricator early on in the process.

Sure but what we are talking about here is showing or hiding a navbox right now. This is not a breaking change and we can easily show them again if necessary.

It's important to remember this is the output of a template created by editors. If they were to make it mobile friendly we would have to accommodate it in whatever form. We should assume this will happen at some point, but hiding it now doesn't impact that. With respect to the discussion about structured data, I don't think we can ever think of arbitrary HTML as structured data in the same way we can think of references which are abstracted and the result of a "magic word" provided by Extension:Cite not random HTML.

For your records:

  • We've hidden navboxes for at least 4 years because they does not display well on mobile
  • http://en.m.wikipedia.org/wiki/Template:Navbox already documents that it is hidden on mobile so this is known to editors
  • T124168 captures the issue with all it's history and blockers on web

Sorry for my frustration but I feel like this has been spoken about numerous times at length and it just points out that we have room for improvement in recording and sharing decisions in a central place (cc @phuedx).