Page MenuHomePhabricator

MinervaNeue auto image scaling causes rendering reflows
Closed, ResolvedPublic

Description

When reading an article over a mobile connection (Good 2G or Regular 3G) the lead section of the article often exhibits a re-render for images in the infobox.

This despite MediaWiki core providing fixed width and height attributes on the image that are supposed to let the browser reserve the appropiate space.

First paint:

Screen Shot 2016-04-04 at 22.50.56.png (1,866×1,148 px, 296 KB)

Last major paint:

Screen Shot 2016-04-04 at 22.50.51.png (1,861×1,148 px, 837 KB)

This is caused by Miverva style rules for auto scaling that are supposed to prevent certain bugs and to ensure images with a fixed dimension larger than the mobile device, it is downscaled.

.content a > img {
    max-width: 100% !important;
    height: auto !important;
}
	// Prevent inline styles on images in wikitext
	// Note we restrict to img's to avoid conflicts with VisualEditor shields
	// See bug 62460
	a > img {
		// make sure that images in articles don't cause a horizontal scrollbar
		// on small screens
		max-width: 100% !important;
		height: auto !important;
	}

Event Timeline

We've chatted in standup and those CSS rules are to make images not expand more than the max width of the viewport (which in a mobile device is thin).

Ideally we would remove the height: auto part, but then the images that are wider than the viewport are squished, see example: http://jsbin.com/pivapenije/edit?html,css,output

Let's figure out a solution. Thanks for raising the bug @Krinkle

We've left it in triage without priority for now to review it again tomorrow and we'll set the priority then (IMO this should be high).

So looks like we could use the padding-bottom hack but that would mean that we'd have to add an inline style for each image, which would bloat the HTML. Or are image ratios pre-defined? If yes, we could just create a handful of classes to cater for different sized images.

It is indeed a really ugly hack. Here's another article (there are a bunch more) talking about the same method related specifically to the problem we're having.

If we add pre-defined ratios that's still going to cause reflows, even if smaller.

I've been doing some research and I'd favor adding behavior in MobileFrontend's MobileFormatter to add a wrapper per image like <div style='padding-bottom: ($height/$width)'> unless we find a better solution even if it adds crap to the markup (but it would make no FOUCs and responsive width images work fine) and it is an ugly solution.

Also, it is the recommended technique on Google PageSpeed Insights docs (last bullet point)

  • For images, this article provides a nice overview on how to serve responsively-sized images without incurring unnecessary page reflows during rendering.

I've been testing it out and it works fine, except for when you want to set a width for the image (which we always want to do). See http://jsbin.com/wohaviteve/edit?html,css,output

When the viewport is bigger than the image, and the image has to stay smaller, then the hack doesn't work, the responsive-embed grows taller than it should because the height is based on the width of the parent (see the black box growing below the img) in previous example.

For preserving the original image width instead of growing to 100% always, we have to add another wrapper with the width of the img and the max-width, like this: http://jsbin.com/pewimapena/edit?html,css,output

Then it works exactly as we would want, but it is as ugly as it can get...

I'm at a loss here, please chime in, I'm starting to think all this hacks are not worth it.

Copy of the last jsbin (commented) just in case:

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>responsive image</title>
</head>
<body>
  asdf
  <!-- Set width of original image here -->
  <div class='responsive-embed' style='width: 300px;'>
    <!-- Set aspect-ratio here (w/h)*100 -->
    <div style='padding-bottom: 66.6%;'>
      <!-- Original image -->
      <img src='http://placekitten.com/300/200' />
    </div>
  </div>
  asdf
</body>
</html>
/* Container with the width */
.responsive-embed {
  max-width: 100%;
}

/* Wrapper to preserve aspect ratio when not loaded */
.responsive-embed>div {
  position: relative;
  height: 0;
  overflow: hidden;
  background-color: black;
}

/* Expand embed to wrapper */
.responsive-embed img {
  position: absolute;
  top: 0;
  left: 0;
  width:100%;
}

How about something like this? http://jsbin.com/zibexezoze/edit?html,css,output

This is truly responsive in a sense that if the screen size is big, then image scales up too. The downside is that the smaller resolution images will get blurry.

Edit: I see Joaquin you didn't want to grow the image width 100% in the above comment.

Edit 2: I think I've found it: http://jsbin.com/zaxifamuyo/edit?html,css,output

I'm still thinking about this and collecting my thoughts but my opinion is we should not be hacking a fix for this using headings or containers. I ask that no one rushes to solution mode (i.e. patch submitting for MobileFrontend) for this until we have agreement on the approach. Please do feel free to keep pastebinning :)

the lead section of the article often exhibits a re-render for images in the infobox

Can we have some example article's in the wild?
If I'm not mistaken, if a thumbnail is smaller than the screen device a re-render will not occur - it is only where images are greater than the device width that this re-render happens. I'm keen to understand how often this repaint occurs before evaluating solutions.

From what I see, most infoboxes have images smaller than 320px which is a typical device width.

Would a better solution not be to limit the widths of thumbnails we send in the first place?

Jhernandez raised the priority of this task from Medium to High.Jul 27 2016, 4:55 PM
dr0ptp4kt changed the task status from Open to Stalled.Aug 4 2016, 3:18 PM

Engineers need to get together and talk about this as team.

dr0ptp4kt added subscribers: Nirzar, dr0ptp4kt.

@Nirzar, what UX do you recommend in an ideal world? Any change? Or no change? Please let Reading Web engineering know in case further technical explanation is required.

This is an engineering problem. The obvious UX recommendation is not to have a lash of unstyled content.

Jdlrobson renamed this task from MobileFrontend auto image scaling causes rendering reflows to MinervaNeue auto image scaling causes rendering reflows.Jul 13 2017, 5:39 PM
Jdlrobson edited projects, added: MinervaNeue; removed: MobileFrontend.
Aklapper changed the task status from Stalled to Open.Nov 1 2020, 10:44 PM

The previous comments don't explain who or what (task?) exactly this task is stalled on ("If a report is waiting for further input (e.g. from its reporter or a third party) and can currently not be acted on"). Hence resetting task status, as tasks should not be stalled (and then potentially forgotten) for years for unclear reasons.
If specific people need to provide input here or discuss and decide, then those people should be asked to provide input, discuss and decide.

(Smallprint, as general orientation for task management:
If you wanted to express that nobody is currently working on this task, then the assignee should be removed and/or priority could be lowered instead.
If work on this task is blocked by another task, then that other task should be added via Edit Related Tasks...Edit Subtasks.
If this task is stalled on an upstream project, then the Upstream tag should be added.
If this task requires info from the task reporter, then there should be instructions which info is needed.
If this task needs retesting, then the TestMe tag should be added.
If this task is out of scope and nobody should ever work on this, or nobody else managed to reproduce the situation described here, then it should have the "Declined" status.
If the task is valid but should not appear on some team's workboard, then the team project tag should be removed while the task has another active project tag.)

Jdlrobson changed the task status from Open to Stalled.EditedNov 24 2020, 6:02 PM

In short, this is an extremely complicated and large problem which originates in user generated content. Our styles exist to override inline styles.

I see this as a community problem, which involves solving the problem in the template.

The issue is valid but currently inactionable without a plan for working with the community to address these problems (stalled on Reading web product owner AND community liasons talking)

What is unusual about these images compared to "regular" thumbnails?

Do they use uncommon wikitext capabilities and/or otherwise produce unusual HTML or unexpected attributes?

What is causing the aspect ratio to not match at all? The images in question are not stretched out, so the width/height ratio is accurate and should suffice the same it does for other responsibe images?

If you could change anything you want, what would be a way, in principle, an example of how this could work correctly at all?

What is unusual about these images compared to "regular" thumbnails?

My understanding of the core problem statement was as follows:

  • Editors explicitly define a preferred width and height for images. This is often not written with mobile in mode, for example in this example this is 500x230....
  • ... for mobile phone resolutions an image with width 500px won't fit, so in lieu of a better technical solution, Minerva makes the decision to scale it down using max-width: 100% !important; unfortunately this results in a squished image...
  • ... so height: auto is applied... this results in a change of height for the image....
  • ... which results in a reflow in some(?) browsers

I can't replicate the exact example from the description of this task for infoboxes, so it's possible some site styles may have been interfering or some browser changes have happened when this was originally posted. Do you know of a more recent example preferably with oldid so it can persist as an example?

There is a little bit of overlap with T197188.

Modern browsers support aspect-ratio: attr(width) / attr(height); which should fix this problem. And it seems browsers that support this, already set this in their user agent styles by default (Safari, Firefox, Chrome at least). So this problem should no longer occur with these browsers.

So likely fixed after Safari 15 (sept 2021), FF 89 (June 2021) and Chrome 88 (Jan 2021)

This despite MediaWiki core providing fixed width and height attributes on the image that are supposed to let the browser reserve the appropiate space.

The explanation for this is that as soon as you set CSS, you technically override the attribute. So from the renderer perspective, we had images with: height: auto (aka unknown), width: fixed pixels, max-width: 100%. But as we did not have final dimensions of the image (since it isn't downloaded yet and attributes can lie), that makes it impossible to calculate the dimensions...

This problem is fixed in Safari 15, Chrome 88, FF89 and later, with the introduction of intrinsic aspect-ratio's for replaceable elements (if you use width/height properties).

We could consider adding a media @supports query around the max-width: 100% statements, but honestly I think that at least on mobile that is not worth it.

Jdlrobson claimed this task.

Yeh I cannot replicate this any more. Thanks for the update @TheDJ