956192 - Replace data: uris in shared/

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia, defect)

Description

[[:fabrice] Fabrice Desré]

data: uris cause memory usage issues, so we must avoid them. Currently they are found in:

shared/js/opensearch.js:
shared/style/confirm/content_details.html:
shared/style/confirm/content.html:
shared/style/input_areas/index.html:
shared/style_unstable/lists/index.html:
shared/style_unstable/tabs/index.html:
shared/style_unstable/toolbars/index.html:
shared/style_unstable/progress_activity/index.html:
shared/style_unstable/drawer/index.html:
shared/style_unstable/seekbars/index.html

Several of these are used by the email and clock apps so that is already a real issue.

Comment 1

[[:fabrice] Fabrice Desré]

Pavel, are you owning these files?

Comment 2

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Hey Fabrice,

the uris in to "shared/style/" and "shared/style_unstable/" are only for reference/demo for the [BB] we can change them with real images if needed (just ping me if I can help with this).

the uris in to "shared/js/opensearch.js" I don't have idea who own them.

Hope this helps :)

Comment 3

[[:fabrice] Fabrice Desré]

(In reply to Pavel Ivanov [:ivanovpavel] from comment #2)
> Hey Fabrice,
> 
> the uris in to "shared/style/" and "shared/style_unstable/" are only for
> reference/demo for the [BB] we can change them with real images if needed
> (just ping me if I can help with this).

Well, the email and clock apps are importing these shared files. Can you move the images into their own files?

> the uris in to "shared/js/opensearch.js" I don't have idea who own them.

ok. kgrandon, do we still use this file?

Comment 4

[Kevin Grandon :kgrandon]

(In reply to Fabrice Desré [:fabrice] from comment #3)
> ok. kgrandon, do we still use this file?

The file is no longer used. I have removed it: https://github.com/mozilla-b2g/gaia/commit/fc6080c035c99c1483da693265eac8755e1d8255

Comment 5

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia/master

Comment 6

[:gerard-majax]

(In reply to Fabrice Desré [:fabrice] from comment #0)
> data: uris cause memory usage issues, so we must avoid them. Currently they
> are found in:
> 
> shared/js/opensearch.js:
> shared/style/confirm/content_details.html:
> shared/style/confirm/content.html:
> shared/style/input_areas/index.html:
> shared/style_unstable/lists/index.html:
> shared/style_unstable/tabs/index.html:
> shared/style_unstable/toolbars/index.html:
> shared/style_unstable/progress_activity/index.html:
> shared/style_unstable/drawer/index.html:
> shared/style_unstable/seekbars/index.html
> 
> Several of these are used by the email and clock apps so that is already a
> real issue.

Just out of curiosity, do you have figures on the impact ?

Comment 7

[[:fabrice] Fabrice Desré]

Comment on attachment 8355901 [details] [review]
patch for Gaia/master

I'd like a gaia reviewer to look at that instead of me!

Comment 8

[[:fabrice] Fabrice Desré]

(In reply to Alexandre LISSY :gerard-majax from comment #6)
> 
> Just out of curiosity, do you have figures on the impact ?

In general, we pay the price of the base64 encoding (33% overhead) and the fact that JS strings use 16bits characters (100% overhead). See bug 902559 and dependents for gory details on how that already caused us trouble.

Comment 9

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Comment on attachment 8355901 [details] [review]
patch for Gaia/master

Hey Kevin,

can you take a look?

Comment 10

[Kevin Grandon :kgrandon]

The referenced files in Comment 0 are actually for demo purposes, and not part of the build. If we have a hard requirement of having no references, then we should remove them - but if not I would be ok with leaving them.

The only two places I could find within the system that is actively using them might be here:

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/accessibility/accessibility.css#L2
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/system/system.css#L223

Speaking of, that line in system.css is a duplicate, and I will file a bug to fix it.

Comment 11

[[:fabrice] Fabrice Desré]

Kevin, the email app is using them, so the "demo purposes" kind of morphed into "let's use them!".

email/shared/style_unstable/lists/index.html:43:
email/shared/style_unstable/lists/index.html:49:
email/shared/style_unstable/lists/index.html:53:
email/shared/style_unstable/tabs/index.html:38:
email/shared/style_unstable/tabs/index.html:42:
email/shared/style_unstable/tabs/index.html:46:
email/shared/style_unstable/toolbars/index.html:26:
email/shared/style_unstable/toolbars/index.html:30:
email/shared/style_unstable/toolbars/index.html:34:
email/shared/style_unstable/toolbars/index.html:39:
email/shared/style_unstable/toolbars/index.html:43:
email/shared/style_unstable/progress_activity/index.html:47:
email/shared/style_unstable/drawer/index.html:53:
email/shared/style_unstable/seekbars/index.html:45:
email/shared/style/confirm/content.html:34:
email/shared/style/input_areas/index.html:32:

Comment 12

[Kevin Grandon :kgrandon]

Fabrice - I am fairly certain that these files are simply a relic of the email build system, which basically copies the entire shared folder into the email application.

All of the email content should be located in: apps/email/js/cards/

I am also quite confused by the pull request in this bug - it doesn't seem to change any data URIs. Anyway - I haven't found evidence of usage of data URIs in production code, though it does appear in grep results. I'm going to close this for now as WONTFIX (those are just demo pages), but if you think this is wrong, please reopen.