Page MenuHomePhabricator

On File: page, for size preview "337 × 599 pixels", "Other resolutions" should not offer non-existing "337 × 600 pixels"
Open, Needs TriagePublicBUG REPORT

Description

On https://commons.wikimedia.org/wiki/File:Historical_Power_Tower_Foundation.jpg
we note

Size of this preview: 337 × 599 pixels. Other resolutions: … 337 × 600 pixels …

However both …599 and …600 hyperlink to the same file!

And which resolution is it? Testing shows: 599!

Therefore there is in fact no "other resolution 337 × 600 pixels". It should be
eliminated from the list!

(Let's assume the bug is due to some tough-to-solve numerical rounding
error. OK, no problem. Just, when composing the list, simply compare the
filenames of each list candidate with the filename of the default
thumbnail. If there is an exact match, just don't add it to the list.)

Same exact bug occurs also on
https://commons.wikimedia.org/wiki/File:Historical_Power_Tower_Foundation_Corner.jpg

Event Timeline

Jidanni created this task.Jan 16 2021, 3:35 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 16 2021, 3:35 AM
Jidanni added a comment.EditedJan 16 2021, 3:39 AM

One might say "Well, the program is just trying to give an easy-to-read full list of resolutions. Don't be so harsh on them for using the word "other."
Nope. As we see in https://commons.wikimedia.org/wiki/File:Historical_Power_Tower_Foundation_2.jpg the bug does not occur.

Aklapper renamed this task from There is actually no "other resolution 337 × 600 pixels". It should be eliminated from the list. to On File: page, for size preview "337 × 599 pixels", "Other resolutions" should not offer non-existing "337 × 600 pixels".Jan 16 2021, 9:19 AM

MediaWiki generates links to thumbnails that fit within certain size boxes, by default, those boxes are

$wgImageLimits = [
	[ 320, 240 ],
	[ 640, 480 ],
	[ 800, 600 ],
	[ 1024, 768 ],
	[ 1280, 1024 ]
];

This issue occurs with this image at the 800x600 size, which is also the default thumbnail size on file description pages. Because this image is in portrait orientation, the height limits. This bug will only occur with portrait images (height > width)

The main thumbnail uses the following logic to round the size when height limits (from getDisplayWidthHeight):

$newwidth = floor( $width * $maxHeight / $height );
$height = round( $height * $newwidth / $width );
$width = $newwidth;
// Note that $height <= $maxHeight now, but might not be identical
// because of rounding.

The note in the comment is exactly what is happening here. The actual height ends up being 599.

There is logic to exclude thumbnail links that match the displayed size in openShowImage:

if ( ( ( $size[0] <= $width_orig && $size[1] <= $height_orig )
		|| ( $this->displayImg->isVectorized()
			&& max( $size[0], $size[1] ) <= $wgSVGMaxSize )
	)
	&& $size[0] != $width && $size[1] != $height
)

If the width or height match the main thumbnail, the link is not displayed. At this point, the main thumbnail size has been appropriately calculated, but the thumbnail link sizes are still the maximum bounding box sizes (800x600). Because 337 != 800 and 599 != 600, the thumbnail link is generated. Something in makeSizeLink will make the calculation later, but openShowImage only sees the resulting HTML. That means it wouldn't be easy to fix this bug by just comparing filenames.

The only solution I can think of is copying the bounding box size logic to yet another place in the code so the true sizes can be calculated first. That doesn't sound like a great idea to me.

Jidanni added a comment.EditedJan 17 2021, 5:13 AM

That means it wouldn't be easy to fix this bug by just comparing filenames.

Well all I know is in the two cases I posted a filename comparison, done at the very last stage, would have avoided the duplicate link being added to the page.

And maybe having the filenames reflect both dimensions, not just X,

…/337px-…

would help. (But then cause a lot of pages that have previously embedded them to fail.)

That means it wouldn't be easy to fix this bug by just comparing filenames.

Well all I know is in the two cases I posted a filename comparison, done at the very last stage, would have avoided the duplicate link being added to the page.

At the stage where a comparison could happen, we don't have a filename, we have the entire translated HTML of the link. Comparing filenames would mean parsing that HTML.

And maybe having the filenames reflect both dimensions, not just X,

…/337px-…

would help. (But then cause a lot of pages that have previously embedded them to fail.)

That would not fix this issue, and create new ones.

However, thinking about it, we could just compare bounding boxes to bounding boxes instead of final image size to bounding boxes. The scaling calculation is deterministic, so with the same inputs you will get the same output.

Change 656641 had a related patch set uploaded (by AntiCompositeNumber; owner: AntiCompositeNumber):
[mediawiki/core@master] page: Compare thumbnail bounding box sizes to bounding box size

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