Page MenuHomePhabricator

viewBox="0,0,620,472" should be the same as viewBox="0 0 620 472"
Closed, ResolvedPublic

Description

https://commons.wikimedia.org/wiki/File:WikiImplementationBug_T194192.svg
Librsvg renders it correctly, but the image ist wrong embedded in the html:

html
<img alt="Vorschaubild der Version vom 8. Mai 2018, 16:47 Uhr" src="https://upload.wikimedia.org/wikipedia/commons/thumb/archive/b/bd/20180508164819%21Test.svg/120px-Test.svg.png" width="120" height="120" data-file-width="512" data-file-height="512" />

but it should be:

html
<img alt="Vorschaubild der Version vom 8. Mai 2018, 16:47 Uhr" src="https://upload.wikimedia.org/wikipedia/commons/thumb/archive/b/bd/20180508164819%21Test.svg/120px-Test.svg.png" width="120" height="91" data-file-width="620" data-file-height="472">

or it could also be:

html
<img alt="Vorschaubild der Version vom 8. Mai 2018, 16:47 Uhr" src="https://upload.wikimedia.org/wikipedia/commons/thumb/archive/b/bd/20180508164819%21Test.svg/120px-Test.svg.png">

How it is impemented:

How it should be impemented:

Examples:

Event Timeline

It's unclear to me where exactly to see the problem. Is this about the "File history" section?

How is the task's title related to the issue? Asking as viewBox is nowhere mentioned in the task description.

Restricted Application added projects: Commons, Multimedia. · View Herald TranscriptMay 8 2018, 6:37 PM
JoKalliauer updated the task description. (Show Details)May 8 2018, 6:44 PM

Thanks, so I guess this is not about the "File history" section.
Which web browser is this about? Have you tried bypassing your browser cache?

As you can see in https://commons.wikimedia.org/wiki/File:WikiImplementationBug_T194192.svg#Test-Area it is a problem of the implementation.

The problem is if viewBox uses as list-separator comma and not spaces librsvg knows the size, but the Implementation not, therefore it just uses the default 512x512

Using commas and not spaces is a workaround ( https://github.com/RazrFalcon/svgcleaner/issues/113 ) for https://phabricator.wikimedia.org/T32033 , where you have to use commas and not spaces (the other way round), see https://commons.wikimedia.org/wiki/Librsvg_bugs#stroke-dasharray

@Aklapper: It is not a problem of browser cache, it is a problem that the Website does not include the png correct.
(Ps: I deleted several browser cache, and the problem still occurs in Google-Chrome, Firefox, Internet Explorer and Microsoft Edge)

How it is implemteted:

html
<img alt="Vorschaubild der Version vom 8. Mai 2018, 18:33 Uhr" src="https://upload.wikimedia.org/wikipedia/commons/thumb/b/bd/Test.svg/120px-Test.svg.png" width="120" height="120" data-file-width="512" data-file-height="512" />

How it should be implemted

html
<img alt="Vorschaubild der Version vom 8. Mai 2018, 18:33 Uhr" src="https://upload.wikimedia.org/wikipedia/commons/thumb/b/bd/Test.svg/120px-Test.svg.png" width="120" height="91" data-file-width="512" data-file-height="390" />
JoKalliauer renamed this task from viewBox="0,0,618,453" should be the same as viewBox="0 0 618 453" to viewBox="0,0,620,472" should be the same as viewBox="0 0 620 472".May 8 2018, 7:16 PM
JoKalliauer updated the task description. (Show Details)
Glrx added a comment.EditedMay 10 2018, 4:07 PM

JoKalliauer is correct.

The SVG specification allows viewBox to be space separated or comma separated, so "0 0 620 472" or "0,0,620,472" would be legal SVG viewBox attributes.

Most SVG files use space-separated viewBox attributes. JoKaulliauer is running some scripts on SVG files. In an effort to workaround librsvg bugs, he's used an option that turns viewBox space-separated lists into comma-separated lists. Hence this bug was discovered.

MediaWiki must learn the size of the svg file in order to scale it properly. MW presumably does that by parsing the SVG file's root element to learn the image's dimensions. The viewBox attribute should be the authority for the dimensions; if that attribute is not present, MW would probably look for the width and height attributes. (Those attributes default to 100% and should not be set to absolute sizes.) Failing to find either, MW would probably assume 512x512.

The bug would be MW finds the viewBox attribute, but cannot parse one with commas, so the default 512x512 remains.

That MW believes the file is 512x512 is shown by

https://commons.wikimedia.org/wiki/File:WikiImplementationBug_T194192.svg

scroll down to file history, look at the dimensions column, which is 512x512

scroll down to the bottom metadata section, and the width and height are each "100%"

Right click on the SVG image to open it in another tab, right click to view source, and the root svg element has

viewBox="0,0,620,472" (no width or height attributes, so they default to 100%).

Glrx added a comment.May 11 2018, 12:50 AM

https://doc.wikimedia.org/mediawiki-core/master/php/SVGMetadataExtractor_8php_source.html

line 328: $viewBox = preg_split( '/\s+/', trim( $this->reader->getAttribute( 'viewBox' ) ) );
line 329: if ( count( $viewBox ) == 4 ) {

Is https://commons.wikimedia.org/wiki/File:Librsvg-svgfont-test.svg the same bug? (The picture seems to be rendered correctly (except for the unknown font-characters).

Vvjjkkii renamed this task from viewBox="0,0,620,472" should be the same as viewBox="0 0 620 472" to 3cdaaaaaaa.Jul 1 2018, 1:11 AM
Vvjjkkii triaged this task as High priority.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed a subscriber: Aklapper.
JoKalliauer renamed this task from 3cdaaaaaaa to viewBox="0,0,620,472" should be the same as viewBox="0 0 620 472".Jul 1 2018, 7:18 AM
JoKalliauer updated the task description. (Show Details)
JoKalliauer added a subscriber: Aklapper.
CommunityTechBot raised the priority of this task from High to Needs Triage.Jul 5 2018, 6:38 PM
Glrx added a comment.EditedJul 5 2018, 8:58 PM

No matter what the priority is, it should be a one line fix to the regex at line 328:

https://doc.wikimedia.org/mediawiki-core/master/php/SVGMetadataExtractor_8php_source.html
line 328: $viewBox = preg_split( '/\s+/', trim( $this->reader->getAttribute( 'viewBox' ) ) );

the trim flushed leading and trailing spaces, so we need not worry about them.

If PHP regexes are greedy, then the regex /\s*[\s,]\s*/ should do the trick:

Find all leading spaces (\s*), find a last space or a comma ([\s,]), find any spaces following the comma (\s*).

So the line should be
line 328: $viewBox = preg_split( '/\s*[\s,]\s*/', trim( $this->reader->getAttribute( 'viewBox' ) ) );

Glrx added a subscriber: TheDJ.Aug 10 2018, 5:55 AM

Adding TheDJ because this is similar to T201274

Change 452013 had a related patch set uploaded (by TheDJ; owner: TheDJ):
[mediawiki/core@master] SVG: Allow , as separator in viewBox attribute value

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

TheDJ claimed this task.Aug 10 2018, 10:18 PM

Change 452013 merged by jenkins-bot:
[mediawiki/core@master] SVG: Allow , as separator in viewBox attribute value

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

TheDJ closed this task as Resolved.Aug 10 2018, 10:58 PM
TheDJ removed a project: Patch-For-Review.