Page MenuHomePhabricator

Long captions overflow their parent
Open, MediumPublic3 Estimated Story PointsBUG REPORT

Description

List of steps to reproduce (step by step, including full links if applicable):

What happens?:
The caption overflows the width of the image. This is particularly visible in production on iPad article views.

What should have happened instead?:
The width pf the caption should be restrained by the size of the image

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc.:

Screenshot 2022-04-04 at 11.34.40.png (332×771 px, 79 KB)

ipad imagecaption bug.jpg (929×2 px, 120 KB)

This is because of the figure/figcaption layout used by Parsoid. Adding the following to the styling would fix the problem:

figure {
  display: table; /* Get sizing from the img */
}
figcaption {
  display: table-caption;
  caption-side: bottom;
}

Ideally, we'd use min-content, but browser support is only 70% percent globally

figure {
    width: -webkit-min-content;
    width: -moz-min-content;
    width: min-content;
}

Screenshot 2022-04-04 at 11.45.48.png (381×705 px, 98 KB)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
TheDJ updated the task description. (Show Details)
This comment was removed by Arlolra.

Yeah, so looking at /html/ instead of /mobile-html/, there's no issue.

Oh, I'm being bit silly. This is already in production on enwiki so it's not quite the same as the changes being rolled out for T51097

This should probably be fixed in,
https://github.com/wikimedia/mobileapps/blob/master/private/styles/mobileapp/figure.less

Ideally, we'd use min-content, but browser support is only 70% percent globally

figure {
    width: -webkit-min-content;
    width: -moz-min-content;
    width: min-content;
}

It wouldn’t work in that 70% either:

Screenshot from 2022-04-05 00-53-07.png (420×1 px, 140 KB)

The image is allowed to shrink (rSMIN resources/skins.minerva.base.styles/content/images.less:52-54 (at 73749969d646), so min-width in this case means the longest word, not the image width.

Which it all imports from minerva it seems ? So really it should be fixed in minerva, and then pulled.

Change 783442 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/skins/MinervaNeue@master] <figure> should not be larger than content size of image

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

Jdlrobson subscribed.

Minerva is a bit of a context switch for me right now so I can't review this today. Adding to our sprint board, but we're operating a bit of a skeleton crew this week, so apologies in advance if we're slow to review this one.

ovasileva triaged this task as Medium priority.Apr 21 2022, 11:12 AM

So I tried to test this with the MobileFrontendContentProvider extension

$wgMFContentProviderClass = 'MobileFrontendContentProviders\\ParsoidContentProvider';

And navigated to /w/index.php?title=List_of_Choate_Rosemary_Hall_alumni&useskin=minerva&mobileaction=toggle_view_mobile
The figure is rendered as:

<figure typeof="mw:Image/Thumb" id="mwOQ"><a href="./File:Glenn_Close_-_Guardians_of_the_Galaxy_premiere_-_July_2014_(cropped).jpg" class="mw-file-description" id="mwOg"><img resource="./File:Glenn_Close_-_Guardians_of_the_Galaxy_premiere_-_July_2014_(cropped).jpg" src="//upload.wikimedia.org/wikipedia/commons/thumb/7/72/Glenn_Close_-_Guardians_of_the_Galaxy_premiere_-_July_2014_%28cropped%29.jpg/116px-Glenn_Close_-_Guardians_of_the_Galaxy_premiere_-_July_2014_%28cropped%29.jpg" decoding="async" data-file-width="1470" data-file-height="2168" data-file-type="bitmap" srcset="//upload.wikimedia.org/wikipedia/commons/thumb/7/72/Glenn_Close_-_Guardians_of_the_Galaxy_premiere_-_July_2014_%28cropped%29.jpg/175px-Glenn_Close_-_Guardians_of_the_Galaxy_premiere_-_July_2014_%28cropped%29.jpg 1.5x, //upload.wikimedia.org/wikipedia/commons/thumb/7/72/Glenn_Close_-_Guardians_of_the_Galaxy_premiere_-_July_2014_%28cropped%29.jpg/233px-Glenn_Close_-_Guardians_of_the_Galaxy_premiere_-_July_2014_%28cropped%29.jpg 2x" id="mwOw" width="116" height="172"></a><figcaption id="mwPA"><a rel="mw:WikiLink" href="./Glenn_Close" title="Glenn Close" id="mwPQ">Glenn Close</a> '65</figcaption></figure>

and looks fine:

Screen Shot 2022-04-29 at 5.03.02 PM.png (1×930 px, 285 KB)

Perhaps the app is not loading the styles inside mediawiki core?
https://github.com/wikimedia/mediawiki/blob/master/resources/src/mediawiki.skinning/content.media-common.less#L68

Note: You need to enable $wgUseContentMediaStyles = false; locally to see those styles applying in Minerva.

Change 787934 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/services/mobileapps@master] [Do Not Merge] Add newer figure and figcaption styles

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

Change 783442 abandoned by TheDJ:

[mediawiki/skins/MinervaNeue@master] <figure> should not be larger than content size of image

Reason:

not needed

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

Change 787935 had a related patch set uploaded (by TheDJ; author: TheDJ):

[mediawiki/services/mobileapps@master] Fix figure and figcaption styling

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

ok, i tried to do it the 'right' way, but it seems that the mobile apps figure HTML structure is not compatible with the core styling and I gave up and just slapped it at the end of parsoid.less

Jdlrobson added a subscriber: SLopes-WMF.

@SLopes-WMF could your team please prioritize the code review here? This is now out of scope for web team.

Change 787935 merged by jenkins-bot:

[mediawiki/services/mobileapps@master] Fix figure and figcaption styling

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

Jgiannelos claimed this task.

I reverted these changes here
Revised issue from the description using Xcode for multiple iPads (see the shots), was unable to reproduce overflow of the parent element.
iPad Pro (12.9 inch)

iPad Pro (12.9 inch).png (1×1 px, 462 KB)

iPad Pro (11 inch)
iPad Pro (11 inch).png (1×1 px, 494 KB)

iPad Pro (9.7 inch)
iPad Pro (9.7 inch).png (1×1 px, 547 KB)

cc: @MSantos , @Jgiannelos

BTW. Not sure why this was no longer reproducible, but I suspect it has to do something with the max-width of the content generated being set these days, which I don't think was there originally, or at least did not have the same dimensions... ? As such the right/left floating image behaviour never kicks in any longer. Of which I'm no certain if I'm a fan, but I guess you could argue that its ok to not make use of horizontal space on tablets.

hmm. did notice another issue though. Even though the floating code never kicks in, it seems that some of the associated margins do... This causes left margin of 1.4em on right aligned images and this forces the images to be aligned off center...

Screenshot 2022-08-15 at 16.18.59.png (790×732 px, 308 KB)
Screenshot 2022-08-15 at 16.20.54.png (452×623 px, 147 KB)

Change 787934 abandoned by TheDJ:

[mediawiki/services/mobileapps@master] [Do Not Merge] Add newer figure and figcaption styles

Reason:

this is an alternate of what broke stuff recently. I doubt this will be more effective. Someone who really understands the code and has more time for follow up than me should work on this.

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

Change 834350 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/skins/MinervaNeue@master] Restrict figure to the size of the media

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

Change 834350 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Restrict figure to the size of the media

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

Change 834364 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/skins/MinervaNeue@wmf/1.40.0-wmf.2] Restrict figure to the size of the media

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

Change 834364 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@wmf/1.40.0-wmf.2] Restrict figure to the size of the media

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

Mentioned in SAL (#wikimedia-operations) [2022-09-22T20:47:42Z] <brennen@deploy1002> Started scap: Backport for [[gerrit:834364|Restrict figure to the size of the media (T305357 T318300)]], [[gerrit:834366|Fix media alignment since disabling wgParserEnableLegacyMediaDOM (T318300)]]

Mentioned in SAL (#wikimedia-operations) [2022-09-22T20:48:02Z] <brennen@deploy1002> brennen and arlolra: Backport for [[gerrit:834364|Restrict figure to the size of the media (T305357 T318300)]], [[gerrit:834366|Fix media alignment since disabling wgParserEnableLegacyMediaDOM (T318300)]] synced to the testservers: mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2022-09-22T20:54:16Z] <brennen@deploy1002> Finished scap: Backport for [[gerrit:834364|Restrict figure to the size of the media (T305357 T318300)]], [[gerrit:834366|Fix media alignment since disabling wgParserEnableLegacyMediaDOM (T318300)]] (duration: 06m 33s)