Page MenuHomePhabricator

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

Description

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:

less/common/ui.less
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:

less/common/pageactions.less
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:
https://bugzilla.wikimedia.org/show_bug.cgi?id=72620

Details

Reference
bz64101

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).
Catrope created this task.Apr 18 2014, 7:58 PM

bingle-admin wrote:

Prioritization and scheduling of this bug is tracked on Mingle card https://wikimedia.mingle.thoughtworks.com/projects/mobile/cards/1941

ori added a comment.Apr 18 2014, 8:26 PM

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

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

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

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

Change 161321 abandoned by Jdlrobson:
WIP: Use mw-ui-icon in beta

Reason:
https://gerrit.wikimedia.org/r/165096

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

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

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

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

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

Florian set Security to None.

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

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

Patch-For-Review

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

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

Jdlrobson closed this task as Resolved.Jan 17 2015, 12:23 AM