Page MenuHomePhabricator

[OTRS]: Article images above the fold not loading at first
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT

Description

How many times were you able to reproduce it?

Every time

Steps to reproduce

  1. Go to article "WYSIWYG" on EN Wikipedia on iPad or go to https://en.wikipedia.org/api/rest_v1/page/mobile-html/WYSIWYG.
  2. Notice the first image

Expected results

Image loads right away

Actual results

Image doesn't load. Only scrolling or backgrounding/foregrounding seems to trigger it into loading.

Screenshots

image0 (2).jpeg (2×1 px, 273 KB)

Environments observed

App version: 6.6.2 (1745)
OS versions: iOS 14 (could also repro on iPad iOS 13 simulator)
Device model: iPad Pro 11 inch
Device language: EN

Affected articles?

  • WYSIWYG

Event Timeline

Hi, it seems that the image is lazy loaded with javascript.
↓ The first time when page was loaded.

<figure class="mw-default-size pcs-widen-image-ancestor">
   <a href="./File:Lorem_Ipsum_-_WYSIWYG_en_Latex_-_tekst_als_paden.svg" class="pcs-widen-image-ancestor">
       <span class="pcs-widen-image-override pcs-lazy-load-placeholder pcs-lazy-load-placeholder-pending" style="width: 640px;" data-class="pcs-widen-image-override" data-src="//upload.wikimedia.org/wikipedia/commons/thumb/d/d6/Lorem_Ipsum_-_WYSIWYG_en_Latex_-_tekst_als_paden.svg/640px-Lorem_Ipsum_-_WYSIWYG_en_Latex_-_tekst_als_paden.svg.png" data-width="640" data-height="480" data-data-file-width="800" data-data-file-height="600">
              <span style="padding-top: 75%;"></span>
       </span>
   </a>
   <figcaption>... content left out ...</figcaption>
</figure>

↓After you scrolled the page,

<figure class="mw-default-size pcs-widen-image-ancestor">
   <a href="./File:Lorem_Ipsum_-_WYSIWYG_en_Latex_-_tekst_als_paden.svg" class="pcs-widen-image-ancestor">
         <img class="pcs-widen-image-override pcs-lazy-load-image-loaded" src="//upload.wikimedia.org/wikipedia/commons/thumb/d/d6/Lorem_Ipsum_-_WYSIWYG_en_Latex_-_tekst_als_paden.svg/640px-Lorem_Ipsum_-_WYSIWYG_en_Latex_-_tekst_als_paden.svg.png" width="640" height="480" data-file-width="800" data-file-height="600">
   </a>
   <figcaption>... content left out ...</figcaption>
</figure>

Check out https://meta.wikimedia.org/api/rest_v1/data/javascript/mobile/pcs for the js source and search for "pcs-lazy-load-placeholder".

These cases don't happen very often because in most pages the first image is wrapped inside a collapsed table (infobox). (At least on the Android app I think the default is to collapse them.)

Having said that, I think it would be good to avoid lazy loading images above the fold. This would also improve performance perception for page content loaded. This would stop other smaller images, like tiny icons, badges, flags, etc., which often come much later in the page content, to be loaded before them.

[See webpagetest: In the enwiki Cat example you can see that Cat_poster_1.jpg is loaded as #19. There are many other requests before it (...Pencil_Icon, ...logo,png). It's even worse on a page that has country flags, like List_of_participating_nations_at_the_Summer_Olympic_Games.]

One problem is to get a good but simple enough definition for what's "above the fold" that can be applied on the server side. Maybe the first image or all images in the lead section? Maybe skip the ones inside infoboxes?

A complicating factor is that the notion of a gallery image is coupled with with lazy loading. That should get decoupled first.

An alternate idea that came to mind was considering to switch the lazy loading implementation to native lazy loading but that doesn't seem to be supported on iOS Safari yet.

MSantos subscribed.

Hi @MSantos , @Tsevener

I've researched this issue. This is how lazy loading applied to the images:

  1. All images intercept by prepareImage() method in MobileHTML class (link)
  2. Inside that method images push into this.lazyLoadableImages array and then passed one by one to finalizeStep -> LazyLoad.convertImageToPlaceholder(this.doc, node); method (link).
  3. When a single image passes to convertImageToPlaceholder, it receives special classes and attributes for lazy loading behavior

The problem is that there are no additional classes applied to these images for interception.
Before the image passes to convertImageToPlaceholder it is just a simple <img> tag with src and dimensions.
I've tried to use the self-defining function just to intercept the first image and apply a custom class to it, but I need more data about images to filter exactly that are first under the edit section header.

So I'm not sure it can be fixed on mobileapps level for now, probably this issue relates to the Templates.

The problem seems to be with the fact that it is using some odd notion of what a gallery image is. I think it is just looking for images that are not "small" to prevent adding lazy loading for thumbnails, icons, etc. (That method name should probably be renamed to clarify its intent).

The solution to this is in what Bernd noted above with his comment: "One problem is to get a good but simple enough definition for what's "above the fold" that can be applied on the server side. Maybe the first image or all images in the lead section? Maybe skip the ones inside infoboxes?"

You need to implement a predicate called "isAbovetheFoldImage" or "imageAboveTheFold" (or such appropriate name) and add it to the check on line 668.

Anyway, but we need to decide on what that means practically. Do @Tsevener or @JMinor have thoughts on that (see Bernd's suggestion that I bolded and include couple lines above).

As I mentioned before, I need some attribute for incoming images to intercept them in prepareImage() method. Also, I've tested a filter function for the first image and this approach did not work well because in this particular article there is an image wrapped by display-none section. Other articles probably can have such kind of hidden images so we are unable to prevent lazy load just for the first image on the page.

As I mentioned before, I need some attribute for incoming images to intercept them in prepareImage() method.

Right, we need some input from Toni and Josh maybe. So, you can unassign yourself from this task. The attribute you are looking for would be the logic in "isAboveTheFoldImage" but we don't know what would work yet.

Also, I've tested a filter function for the first image and this approach did not work well because in this particular article there is an image wrapped by display-none section. Other articles probably can have such kind of hidden images so we are unable to prevent lazy load just for the first image on the page.

Ah, okay. So, yes, we need a bit more clarity on what "above the fold" means. @MSantos @Jgiannelos @Mholloway is this concept already well-documented elsewhere in the MCS / PCS land or does that need to established in consultation with the Apps teams?

Ah, okay. So, yes, we need a bit more clarity on what "above the fold" means. @MSantos @Jgiannelos @Mholloway is this concept already well-documented elsewhere in the MCS / PCS land or does that need to established in consultation with the Apps teams?

AFAIK we don't have this concept documented, my understanding is that the intention was to consider the infobox and collapsable tables as "the fold" and above/inner would be an element that's outside/inside this collapsable table/infobox.

My proposal is to consider every image outside an infobox or collapsible table "above the fold" and remove lazy loading for the ones in the first section that are above the fold.

I think "above the fold" being the images up to and including the first image in the first section of an article sounds good to me. Notably this bug does not seem to happen for images within the infobox, even if we're sending areTablesInitiallyExpanded as true in the setup - those load fine currently. Tagging @JMinor in case he has other thoughts.

Notably this bug does not seem to happen for images within the infobox, even if we're sending areTablesInitiallyExpanded as true in the setup - those load fine currently.

That might be a side-effect of something in the code wrt defaults around collapsed tables/infoboxes. From how you wrote that, presumably that is not a problem, but I consider that a "bug" in the code ... the behavior should be more explicit.

I think "above the fold" being the images up to and including the first image in the first section of an article sounds good to me

Looks like Bernd, Mateus, and you all agree on that definition. So, let's roll with it and tweak it based on playing with it once it lands.

@vadim-kovalenko does that help you implement this now?

Change 675125 had a related patch set uploaded (by Vadim Kovalenko; author: Vadim Kovalenko):
[mediawiki/services/mobileapps@master] Prevent lazy loading for images in the zero and first sections

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

@ssastry Yes, I've uploaded a patch using this approach. In addition, I want to mention that not only section with id=1 can show the issue but with id=0, so I've implemented fix for both cases. Also, I've added additional filtering for collapse tables to make sure that lazy loading is applied to their inner images even if that tables are inside the first two sections.
Also, I've checked this issue in a different articles (this might be useful when testing on local environment):

  • localhost/en.wikipedia.org/v1/page/mobile-html/WYSIWYG - this one from description, image is inside section id=1
  • localhost/de.wikipedia.org/v1/page/mobile-html/Great_Western_Railway - image is inside section id=0
  • localhost/de.wikipedia.org/v1/page/mobile-html/IBM - image inside collapse table inside section id=0

I've fixed all of these cases. If you find anything else regarding this issue, please, let me know.

Change 675125 merged by jenkins-bot:

[mediawiki/services/mobileapps@master] Prevent lazy loading for images in the zero and first sections

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