Page MenuHomePhabricator

CSSMin: Embedded SVG icons are broken in Wikimedia production due to bad mime type
Closed, ResolvedPublic

Description

On MediaWiki.org (1.23wmf9), MW thinks that the appropriate MIME type for SVGs embedded in CSS as data: URIs is application/xml instead of image/svg+xml, causing them not to show up in any browser. It works for me locally.


Version: 1.23.0
Severity: major
URL: https://www.mediawiki.org/wiki/VisualEditor/Basic_example_worksheet#Links

Details

Reference
bz59234

Event Timeline

bzimport raised the priority of this task from to Unbreak Now!.Nov 22 2014, 2:17 AM
bzimport set Reference to bz59234.
bzimport added a subscriber: Unknown Object (MLST).
matmarex created this task.Jan 2 2014, 11:06 PM

For example, this affects the little icons shown next to links; a blank space appears instead.

The empty gap is pretty confusing plus might create side effects (bug 59239); hence supporting blocking next deployment (bug 38865). CC'ing greg.

  • Bug 59249 has been marked as a duplicate of this bug. ***

Since this has worked before, a possible culprit is my commit optimizing the SVGs: If05ee0e0 – lots of things that are supposedly non-essential were removed from the SVGs' source code.

(Quote from bug 59249 comment #1)

https://github.com/wikimedia/mediawiki-core/blob/master/includes/libs/CSSMin.
php#L118-L142

It probably works for us locally because it uses a different function than production, or maybe the underlying library has a different upstream version or configuration of an upstream C-library dependency.

(In reply to comment #4)

Since this has worked before, a possible culprit is my commit optimizing the
SVGs: If05ee0e0 – lots of things that are supposedly non-essential were
removed
from the SVGs' source code.

Yeah, let's find out if putting the doctype back makes it work for mimetype library Wikimedia uses in production.

Otherwise we'll have to abstract mime type detection to make sure that valid XML documents of which the root tag is "<svg>" are detected as SVG (instead of plain XML).

greg added a comment.Jan 3 2014, 3:54 PM

Bartosz: Can you get a patch for re-adding in the doctype on those svgs? If you can get that merged today it'll be on Beta shortly after.

The problem is exhibited on Beta Cluster, see eg:
http://en.wikipedia.beta.wmflabs.org/wiki/User:Greg_%28WMF%29

From #mediawiki a few minutes ago, pasting as it mentions IE difference:
<SVG^> Hey all. Is there already a bug report about the missing arrows in the recent changes at MediaWiki.org with Chrome and FF (IE shows them properly)?

(In reply to comment #6)

Bartosz: Can you get a patch for re-adding in the doctype on those svgs?

I didn't even remove doctypes from most of these files (they haven't had any). I'll leave shotgun debugging to someone who has access to production environment; if you need this fixed immediately, try reverting If05ee0e0.

(In reply to comment #7)

From #mediawiki a few minutes ago, pasting as it mentions IE difference:
<SVG^> Hey all. Is there already a bug report about the missing arrows in the
recent changes at MediaWiki.org with Chrome and FF (IE shows them properly)?

Old IE uses the fallback PNG images, which show up correctly.

(In reply to comment #7)
Not sure anymore if that's the same problem. Filed as bug 59452 instead.

greg added a comment.Jan 3 2014, 4:26 PM

(In reply to comment #8)

(In reply to comment #6)

Bartosz: Can you get a patch for re-adding in the doctype on those svgs?

I didn't even remove doctypes from most of these files (they haven't had
any).
I'll leave shotgun debugging to someone who has access to production
environment; if you need this fixed immediately, try reverting If05ee0e0.

That's what the Beta Cluster is for :).

But you're right, just first random file change I looked at removed the doctype, but the external link svg, which is not shown in that test case on Beta Cluster, didn't have doctype before either.

I'm confused now.

On en.wiki (production), the lock icon next to an external url is a base64 encoded image/png.

On en.betawiki, the lock icon is a base64 application/xml, that is in fact valid/renders if you load it directly. It just doesn't show on the page.

Both viewed with Iceweasel 24.2.

  • Bug 59452 has been marked as a duplicate of this bug. ***
greg added a comment.Jan 4 2014, 12:08 AM

Ori just reverted it via https://gerrit.wikimedia.org/r/#/c/105413/ and pushed it out to wmf9. Looks fixed on the Beta Cluster and mw.org to me.

His commit message:
On my Mac, calling finfo_file on external-link-ltr-icon.svg gets the correct
MIME-type ("image/svg+xml"), but in production I get "application/xml".
This is probably due to the version of fileinfo or libmagic that is deployed on
the cluster. Reverting for now.

Robla made the point that just upgrading libmagic isn't the right/full solution: we shouldn't break backwards compat here that quickly.

(In reply to comment #12)

Ori just reverted it via https://gerrit.wikimedia.org/r/#/c/105413/ and
pushed
it out to wmf9. Looks fixed on the Beta Cluster and mw.org to me.

I don't know what you are looking at on mw.org, but it certainly isn't fixed for me or Mz ;)

I'm looking at the main page on mw.o with FF26 on OSX10.9.1 and it looks fine to me.

(In reply to comment #14)

I'm looking at the main page on mw.o with FF26 on OSX10.9.1 and it looks fine
to me.

Tried recent changes with enhanced recent changes on?

Created attachment 14228
Screenshot of missing arrows from [[mw:Special:RecentChanges]]

(In reply to comment #13)

I don't know what you are looking at on mw.org, but it certainly isn't fixed
for me or Mz ;)

I uploaded a screenshot of how [[mw:Special:RecentChanges]] looks to me at the moment. Perhaps cache?

Attached:

IIRC I was silly enough to optimize the recent changes arrows in a different commit, eh. Sorry about that, patch on the way.

Change 105469 had a related patch set uploaded by Bartosz Dziewoński:
Deoptimize SVG versions of enhanced recent changes collapse/show arrows

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

Done, sorry about the mess. Backport and deploy to taste.

Change 105469 merged by jenkins-bot:
Deoptimize SVG versions of enhanced recent changes collapse/show arrows

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

Change 105477 had a related patch set uploaded by Legoktm:
Deoptimize SVG versions of enhanced recent changes collapse/show arrows

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

(In reply to comment #21)

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

This one is the backport to 1.23wmf9, which someone else will need to merge+deploy.

I'm seeing missing arrows next to "Templates used on this page:" below the edit window on mediawiki.org as well. Example URL: https://www.mediawiki.org/w/index.php?title=Requests_for_comment/Custom_inter-namespace_tabs&action=edit. I'm not sure if this is already covered by the Gerrit changes linked above.

the arrow fixed hasn't been merged and deployed in wmf9, i've tagged reedy on that changeset.

(I hoper we aren't using yet another set of arrows icons for that)

(In reply to comment #24 and comment #25)

Neither of these comments were helpful as a reply to comment 23. Thanks for adding Reedy to the relevant changeset, though.

This seems to have been fixed in file 5.04; however, PHP uses a bundled,
patched copy of the library that was only updated from 5.03 to 5.11 in
12cf930a403d.

Running "git tag --contains 12cf930a403d" in php-src tells me that
affected versions of PHP were 5.3.10 and older, as well as 5.4.0.
Wikimedia runs 5.3.10 in production.

Here's a snippet of the diff:

  • file-5.03/magic/Magdir/sgml 2008-07-26 11:03:55.000000000 -0400

+++ file-5.04/magic/Magdir/sgml 2009-09-19 13:31:35.000000000 -0400
@@ -1,36 +1,59 @@

+# $File: sgml,v 1.24 2009/09/19 17:31:35 christos Exp $

  1. Type: SVG Vectorial Graphics
  2. From: Noel Torres <tecnico@ejerciciosresueltos.com> 0 string \<?xml\ version=" >15 string >\0

->>23 search/400 \<svg SVG Scalable Vector Graphics image
+>>19 search/4096 \<svg SVG Scalable Vector Graphics image
!:mime image/svg+xml

Note that the start offset of the search for "<svg" was dropped from 23 to 19, and the newer, shorter XML declaration was the following 22-character string:

<?xml version="1.0" ?>

So this problem could have been avoided by either adding a DOCTYPE or leaving one more whitespace character in the right place.

(In reply to comment #16)

Created attachment 14228 [details]
Screenshot of missing arrows from [[mw:Special:RecentChanges]]

I can still reproduce MZMcBride's screenshot after enabling "Group changes by page in recent changes and watchlist" on https://www.mediawiki.org/w/index.php?title=Special:Preferences#mw-prefsection-rc

<span class="mw-collapsible-toggle mw-collapsible-arrow mw-enhancedchange…-enhancedchanges-arrow-space mw-collapsible-toggle-collapsed" tabindex="0"></span> is empty but should probably show a collapsible arrow (if I interpret the classes correctly here).

Attached:

greg added a comment.Jan 6 2014, 4:38 PM

(In reply to comment #23)

I'm seeing missing arrows next to "Templates used on this page:" below the
edit
window on mediawiki.org as well. Example URL:
<https://www.mediawiki.org/w/index.php?title=Requests_for_comment/
Custom_inter-namespace_tabs&action=edit>.
I'm not sure if this is already covered by the Gerrit changes linked above.

Ditto.

Also seeing it on the Beta Cluster, eg at http://en.wikipedia.beta.wmflabs.org/w/index.php?title=Dido_Sotiriou&action=edit

The Beta Cluster is already running with https://gerrit.wikimedia.org/r/#/c/105469/ though, ie: even the cherrypick to wmf9 (https://gerrit.wikimedia.org/r/#/c/105477/) won't fix it on mw.org most likely.

So, this issue isn't fixed with any of the already merged (to master) patches.

(In reply to comment #16)

Created attachment 14228 [details]
Screenshot of missing arrows from [[mw:Special:RecentChanges]]
(In reply to comment #13)

I don't know what you are looking at on mw.org, but it certainly isn't fixed
for me or Mz ;)

I uploaded a screenshot of how [[mw:Special:RecentChanges]] looks to me at
the
moment. Perhaps cache?

I don't see that issue on the BetaCluster, but I can confirm on mw.org.

So, this one is indeed fixed with the patches merged to master.

Attached:

Change 105477 merged by jenkins-bot:
Deoptimize SVG versions of enhanced recent changes collapse/show arrows

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

Reedy added a comment.Jan 6 2014, 5:05 PM

(In reply to comment #30)

Change 105477 merged by jenkins-bot:
Deoptimize SVG versions of enhanced recent changes collapse/show arrows
https://gerrit.wikimedia.org/r/105477

Fixed mediawiki.org for me

greg added a comment.Jan 6 2014, 5:08 PM

(In reply to comment #29)

(In reply to comment #23)

I'm seeing missing arrows next to "Templates used on this page:" below the
edit
window on mediawiki.org as well. Example URL:
<https://www.mediawiki.org/w/index.php?title=Requests_for_comment/
Custom_inter-namespace_tabs&action=edit>.
I'm not sure if this is already covered by the Gerrit changes linked above.

Ditto.
Also seeing it on the Beta Cluster, eg at
http://en.wikipedia.beta.wmflabs.org/w/index.
php?title=Dido_Sotiriou&action=edit
The Beta Cluster is already running with
https://gerrit.wikimedia.org/r/#/c/105469/ though, ie: even the cherrypick to
wmf9 (https://gerrit.wikimedia.org/r/#/c/105477/) won't fix it on mw.org most
likely.
So, this issue isn't fixed with any of the already merged (to master)
patches.

I think I mispoke here/wasn't looking at the right thing. I think this one is indeed fixed by the patches.

I think thanks to Reedy's lastest push, everything is back to normal/good. Anyone seeing any other inconsistencies/lack of images?

Both https://www.mediawiki.org/w/index.php?title=Requests_for_comment/Custom_inter-namespace_tabs&action=edit and [[mw:Special:RecentChanges]] look good to me now. Thanks!

Marking this ticket as resolved/fixed. I'll clone this ticket to cover the general issue. Kevin: thanks as always for digging into this. :-)

(In reply to comment #33)

I'll clone this ticket to cover the general issue.

Filed as bug 59744. I only CC'd Kevin. The rest of you can CC yourselves if you'd like.