1566072 - Investigate scope of Preview Icons

Status: RESOLVED FIXED

(DevTools :: Application Panel, task, P1)

Description

[Ola Gasidlo [:ogasidlo] [:ola] OOO - please ping :ladybenko:]

Investigate the scope of Preview Icons in the application panel.

• Which formats does manifest support?
• What do we do with images that are not supported by the manifest, but by the browser. Do we show them?
• Security issues (injecting code through path / images)
• Functionality already in place as the tooltip in inspector is showing images (https://searchfox.org/mozilla-central/source/devtools/client/inspector/markup/views/element-container.js#154-183)

Comment 1

[Ola Gasidlo [:ogasidlo] [:ola] OOO - please ping :ladybenko:]

This is the function the validation is using: https://searchfox.org/mozilla-central/source/dom/manifest/ImageObjectProcessor.jsm#95-98

Comment 2

[Ola Gasidlo [:ogasidlo] [:ola] OOO - please ping :ladybenko:]

Security issues (injecting code through path / images)

As we get the URL of the icon, we can display a preview of the icons in the panel as long as we use the img element, which will e.g. disable all scripts in an SVG. (see example of an use case: http://html5sec.org/test.svg -> inspector)

functionality already in place as the tooltip in inspector is showing images (https://searchfox.org/mozilla-central/source/devtools/client/inspector/markup/views/element-container.js#154-183)

I need to check here if we can use the cached version of the image instead of an additional request to the original file and how, as the inspector is using the original from: https://searchfox.org/mozilla-central/source/devtools/client/inspector/markup/views/element-container.js#161

Comment 3

[Ola Gasidlo [:ogasidlo] [:ola] OOO - please ping :ladybenko:]

Which formats does manifest support?

Supported formats:

• APNG
• GIF
• ICO
• JPEG / JFIF
• SVG
• PNG
• Lossless WebP

Best practise (those we should suggest):

• SVG
• PNG (also fallback)
• Lossless WebP

What do we do with images that are not supported by the manifest, but by the browser. Do we show them?:

The answer here would be as long as we do support the format, we should show them. But we should also show a warning next to it that the format is not best practice.

Sizes

https://searchfox.org/mozilla-central/source/dom/manifest/ManifestIcons.jsm#51-53

// We start trying the smallest icon that is larger than the requested
// size and go up to the largest icon if they fail, if all those fail
// go back down to the smallest