Page MenuHomePhabricator

Lots of background images not using data URI embedding
Closed, ResolvedPublic


When I installed MobileFrontend without exposing /w/extensions/MobileFrontend on my web server, I got 8 404s for various images and lots of stuff was missing from the interface.

Grepping showed that there are lots and lots of background images that aren't embedded because they're not using the mixin.

Example of correct code:

53: #mw-mf-main-menu-button {
54- .background-size( 24px, auto );
55- background-position: 40% 40%;
56- .background-image-svg( 'images/menu/hamburger.svg', 'images/menu/hamburger.png' );
57- }

Example of incorrect code:

81-#ca-edit {

82: background-image: url(images/pagemenu/edit-locked.png);

84- &.enabled {
85: background-image: url(images/pagemenu/edit.png);

I forget what the mixin is for when you don't have both an SVG and a PNG (also, isn't there an SVG for every single icon?), but there is one in MW core and it should be used.

Running ack-grep -Q 'url(' -B 1 extensions/MobileFrontend/less reveals a total of 50 (!!) cases where url() is used without /* @embed */ and without using a mixin.

Version: unspecified
Severity: normal
See Also:


Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 3:17 AM
bzimport set Reference to bz64101.
bzimport added a subscriber: Unknown Object (MLST).

bingle-admin wrote:

Prioritization and scheduling of this bug is tracked on Mingle card

Mobile folks: since you're going to be looking at this in the context of MobileFrontend, I'd really appreciate it if you also grepped for this pattern across all deployed extensions and helped triage or fix any additional occurrences.

Change 161321 had a related patch set uploaded by Jdlrobson:
WIP: Use mw-ui-icon

Change 165096 had a related patch set uploaded by Jdlrobson:
WIP: Use mw-ui-icon in beta

Change 168138 had a related patch set uploaded by Jdlrobson:
Use mw-ui-icon in alpha

Change 168138 merged by jenkins-bot:
Use mw-ui-icon in alpha

Is this bug resolved now? AFAIK, the reason many of those images weren't using embedding is because the embedding system didn't support RTL flipping. So we didn't want to use it for things like arrows. Do you know if that works now?

Not quite. Let me wrap this bug up. Fix on way.

gerritbot added a subscriber: gerritbot.

Change 185558 had a related patch set uploaded (by Jdlrobson):
Use data uris everywhere


Change 185558 merged by jenkins-bot:
Use data uris everywhere