Page MenuHomePhabricator

@embed is not correctly replaced in case of svg files
Closed, ResolvedPublic

Description

I believe we may have found an issue with the way @embed is replaced in less files. According to the comments and code in CSSMin.php (around #313) @embed should not have the fallback for old IE versions added when the file is an SVG. However, we've found that this is exactly what happens - for example, see Vector's generated css.

This is the case in 1.25 and core.

Thanks to @connorshea for noticing something weird in the first place.

Event Timeline

soeb created this task.May 31 2015, 10:15 PM
soeb raised the priority of this task from to Needs Triage.
soeb updated the task description. (Show Details)
soeb added a project: MediaWiki-General.
soeb added subscribers: soeb, connorshea.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 31 2015, 10:15 PM
connorshea updated the task description. (Show Details)May 31 2015, 10:28 PM
connorshea set Security to None.
connorshea updated the task description. (Show Details)
connorshea updated the task description. (Show Details)

Any chance to attach a screenshot that shows the problem for affected browsers (old IE)? Wondering "how bad" this is and don't have an old IE handy...

soeb added a comment.Jun 1 2015, 10:03 AM

Oh, I may have explained this wrong. As I understand the code and comments, the following should happen:

  1. (#316) should happen when the file is not embeddable. This appears to function correctly.
  2. (#319) should happen when the file is embeddable, but also supported by IE6/7 (things like gifs, jpegs). It shouldn't happen for SVGs. Currently, it's also done for SVGs. See below.
  3. (#324) is for all other cases, mainly for handling SVGs - files not supported by IE6/7 anyway, so why have a fallback. Doesn't appear to work/ever happen.

We make use of the @embed solution to have svg icons on our wiki, and it'd be expected only the embed happens, not the IE fallback, as per the comments in the code. However, this is the result.
Note that background-image always has an !ie fallback when using svgs. We're using these mixins, as in Vector, and this also appears to happen in vector.

Aklapper triaged this task as Low priority.Jun 11 2015, 1:07 PM

Thanks for taking a look at the code! Appreciated!

Just in case you feel like cooking a patch, you are very welcome to use developer access to submit a Git branch into Gerrit (or you can also use the Gerrit Patch Uploader).

Change 319280 had a related patch set uploaded (by Bartosz Dziewoński):
CSSMin: Correctly avoid fallbacks when embedding SVG files

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

matmarex claimed this task.Nov 2 2016, 9:30 AM

Change 319280 merged by jenkins-bot:
CSSMin: Correctly avoid fallbacks when embedding SVG files

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

matmarex closed this task as Resolved.Nov 3 2016, 6:58 PM
matmarex removed a project: Patch-For-Review.