Page MenuHomePhabricator

MobileFormatter doesn't run on complex pages and gives no feedback to editors creating confusion
Open, MediumPublic

Description

This diff causes a top-level heading to not get shown as a collapsible section on mobile.

See the discussion on enwiki Village Pump for background.

Event Timeline

Aklapper added a subscriber: RhinosF1.

If this only happens on mobile then this might be in the MobileFrontend extension itself instead of MediaWiki (or maybe maybe in the MinervaNeue skin).

This diff also has the issue meaning the issue is likely caused by the number of files or the size of the file links.

It's not just Minerva, the dropdown icon doesn't show when Vector is set as the mobile skin as well, so it's a MobileFrontend issue.

MobileFrontend sets MFMobileFormatterOptions.maxImages to 1000, and since there's over 1000 images the mobile formatting is not being applied.

MobileFrontend sets MFMobileFormatterOptions.maxImages to 1000, and since there's over 1000 images the mobile formatting is not being applied.

@Jdlrobson wrote the patch that introduced the maxImages and maxHeadings settings and how they're applied (see I877473bbeb8410f4959553076a78f7c51e4b473e) so his input and any context he can give us would be welcome.

However, it seems to me that those settings should only apply to their respective transforms rather than to all transforms, i.e.

includes/MobileFormatter.php
public static function canApply( $text, $options, $transformName ) {
  switch ( $transformName ) {
    case 'HEADINGS':
      return preg_match_all( '/<[hH][1-6]/', $text ) < $options['maxHeadings'];
    case 'IMAGES':
      return preg_match_all( '/<img/', $text ) < $options['maxImages'];
    default:
      return true;
  }
}

For example, I would hope that the MobileFormatter::makeSections transform would run for a page has 1000+ images but only 6 sections.

phuedx triaged this task as Medium priority.Mar 31 2020, 3:45 PM

Prioritised as Medium to match the priority of the task associated with the original change.

phuedx renamed this task from h2 doesn't get tagged as collapsible-heading on mobile to Sections aren't collapsible when there are many images on the page.Mar 31 2020, 3:52 PM

The MobileFormatter cannot handle large pages with lots of DOM nodes. That's why this restriction is in place. The problem is the code that parses the code into a DOM. If we can't do that we can't run any transformations including images or headings.

My suggestion for now would be to give feedback in the editor via a warning message that the page is too complex for mobile formatting.

Jdlrobson renamed this task from Sections aren't collapsible when there are many images on the page to MobileFormatter doesn't run on complex pages and gives no feedback to editors creating confusion.Mar 31 2020, 4:27 PM
Jdlrobson added a project: Readers-Web-Backlog.

(We can also try upping those values as they were set arbitrarily based on articles which were experiencing timeout values. I believe @nray did that analysis.)

At what level is the transformation done? If the large page were broken up into sufficiently small subpages which were then transcluded into a composite top-level page, would that solve the problem?

@RoySmith it's done on the entire finished page so transclusions wouldn't help here. Sorry to not bne able to provide any quick fixes for now :( If you are looking for a short term fix using flag emojis rather than images should help. https://flagpedia.net/emoji

Interesting thought, thanks. I'll suggest it to the OP.

It turns out the OP is having problems accessing phab, but they implemented something very similar and that did indeed resolve the issue.

Jdlrobson edited projects, added MobileFrontend (Tracking); removed MobileFrontend.
Jdlrobson moved this task from Tracking to Team: web on the MobileFrontend board.
Jdlrobson edited projects, added MobileFrontend; removed MobileFrontend (Tracking).

Change 671000 had a related patch set uploaded (by BrandonXLF; owner: BrandonXLF):
[mediawiki/extensions/MobileFrontend@master] Add comment when MobileFormatter can't be applied

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

@RoySmith would an HTML comment that doesnt show up in the editor be sufficient here?

@BrandonXLF perhaps a <div class="error"></div> is best here as this can be considered a parsing errors like the ones we're discussing in T280766