Page MenuHomePhabricator

height=xxx is not respected in timeless
Open, Needs TriagePublic

Event Timeline

Timeless assumes topicon sizes are set appropriately. Maybe this is a stupid assumption, but are you sure you can't just... do that? Set a specific default size in the script?

Timeless assumes topicon sizes are set appropriately. Maybe this is a stupid assumption, but are you sure you can't just... do that? Set a specific default size in the script?

The size is set, but only via height (width is not specified). That...should work, in theory, right?

Ooooooh this is the image overflow stuff crap.

Yeah, no, timeless is breaking it, you're spot on.

Change 685097 had a related patch set uploaded (by Isarra; author: Isarra):

[mediawiki/skins/Timeless@master] Prevent overflowing non-content images from exploding all over the place

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

Change 685097 merged by jenkins-bot:

[mediawiki/skins/Timeless@master] Prevent overflowing non-content images from exploding all over the place

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

Yeah, no, timeless is breaking it, you're spot on.

Thanks for the fix! Unfortunately, I don't think it's going to fix this instance of the issue :-(. Looking through the DOM, the image is #content > .mw-body-content > gadget-groupindicator-global > a > img, so I think the new selector, .mw-body-content a > img, would still catch it and resize.

This really kind of depends on four different patches. Patch 1 and 2, moving the mw-body-content class to what the skin receives, and removing mw-body-content from anything the skin does itself. Patch 3: removing mw-body-content from the indicators. Patch 4: apparently the only one I actually linked here.

I think they've all been merged at this point and will be out at some point, but hell if I know. Also yeah timeless kind of just assumes folks are using 'width' instead of height for images, since height is the normal one to just do automatically and scroll, whereas width becomes a problem at different resolutions. So the specific problem here is that on top of that, this does use height, which makes a lot more sense for being an interface thing and not a content thing. Hence making the rule only hit page content.

Oh, if you do want to fix it ahead of the time in the gadget, set the 'width' to an actual value. Make this one do width=20 instead of height=20. Mind you, heights actually totally make sense for indicators anyway, so it might make more sense to just leave it and see if all these patches fix it...

And yes we totally need a proper solution for sensible use-cases for height-based image sizes in the content too.