Last modified: 2013-11-29 19:07:23 UTC
Through Wikipedia Zero I discovered that the mobile skin was not rendering correctly on Nokia N95 stock browser and various other older mobile browsers. It seems that the reason this breaks is due to the following line which uses an embed url #mw-mf-menu-main li.icon-home a { background-image: /* @embed */ url(images/menu/home.png); } Looking closely in includes/CSSMin.php there is the following line which introduces an IE hack. $replacement .= "{$pre}url({$url}){$post}!ie;"; Is this hack still needed? How can this be worked around? I suspect use of a conditional stylesheet in Internet Explorer to work around this problem would be better? (Note in mobile it is fine for the image in the data uri not to render for a Nokia N95)
For the record I think this sort of thing should be done in javascript. e.g. #mw-mf-menu-main li.icon-home a { background-image: url(images/menu/home.png); } .supportsDataUri #mw-mf-menu-main li.icon-home a { background-image: /* @embed */ url(images/menu/home.png); } Hacks in the startup module seem very nasty.
https://gerrit.wikimedia.org/r/56948 is one suggested approach.
Unfortunately conditional stylesheets for old-IE is hard; you have to use conditional comments in the *HTML source* to <link> or <style>, or load them conditionally from JS, and that's all kinda tricky. But my main concern is just with the embedding syntax. The output _looks wrong_ to me per spec, I'm not sure if it's relying on weird IE bugs instead of using standard fallback methods or what. #mw-mf-menu-main li.icon-loginout a{ background-image:url(data:image/png;base64,...);url(//bits.wikimedia.org/static-1.21wmf12/extensions/MobileFrontend/stylesheets/common/images/menu/lowres/loginout.png?2013-03-27T22:43:20Z)!ie } I can't help but think there should be another 'background-image:' in there for proper syntax. Are the data: URIs confusing the N95 browser or is it the missing "background-image:" or the "!ie"?
This looks completely wrong: background-image: /* @embed */ url(images/menu/home.png); @embed is supposed to be before the property. Either on the previous line or directly before: /* @embed */ background-image: url(images/menu/home.png); This is probably why that background-image: is missing.
(In reply to comment #0) > #mw-mf-menu-main li.icon-home a { > background-image: /* @embed */ url(images/menu/home.png); > } Shouldn't the @embed be before 'background-image'? That how I've always seen it used.
Thanks guys this seems to be what's happening. The regexp should possibly be improved to avoid this sort of misuse. After all this rule could easily be placed have been placed somewhere like MediaWiki:Common.css and break an entire skin on an older device. Seems like a bad code smell. I've abandoned the patchset (although the purist in me still feels it is nasty to have a hack in for a specific browser) - we should at least have a use legacy mode global check in there to allow people to turn it on or off.
Lowering priority. @-annotations need to be used before a declaration or before a selector (to have it apply to all declarations in that rule). Using it within a declaration is invalid use and should be ignored by CSS. It generating invalid CSS is a bug, but even when we fix this bug, it still won't do what you want it do do, which is to embed the image. So, for now, please fix the stylesheets (which you'll have to do anyway).
Agreed. I fixed my usage early today (merge still needed https://gerrit.wikimedia.org/r/#/c/56956/) but we should address the symptom which is giving the impression all is working fine when it actually is not :) Thanks for everyone who pointed out this silly mistake :)
Change 94511 had a related patch set uploaded by Bartosz Dziewoński: Rewrite CSSMin::remap to support multiple url() values in one rule https://gerrit.wikimedia.org/r/94511
Change 94511 merged by jenkins-bot: Rewrite CSSMin::remap to support multiple url() values in one rule https://gerrit.wikimedia.org/r/94511
The above change makes syntax from comment 0 valid, and makes it work like one would expect. It should, however, still be avoided when possible to be compatible with older MediaWiki versions. I also haven't tested how it behaves together with LESS (the compiler might strip the comments).
