1335333 - Update pop-ups have no puzzle icon displayed if there is no icon specified in manifest.json

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Vasilica Tamas, QA [:vtamas]]

[Affected versions]:
Firefox 53.0a2 (2017-01-30)
Firefox 54.0a1 (2017-01-30)

[Affected platforms]:
Windows 10 64-bit
Ubuntu 16.04 32-bit


[Steps to reproduce]:
1.Launch Firefox with clean profile.
2.Create extensions.webextPermissionPrompts and set it to true.
3.Create xpinstall.signatures.dev-root and set it to true.
4.Change extensions.update.background.url to point to dev server and set the update interval (extensions.update.interval) to “1”. 
5.Install the first version of the following webextension: https://addons-dev.allizom.org/en-US/firefox/addon/webext123/versions/
6.Restart the browser. 


[Expected Results]:
The puzzle icon is displayed in update doorhanger for those webextensions that have no icon specified in manifest.json. 


[Actual Results]:
There is no icon displayed in installation pop-up: https://www.screencast.com/t/2LF0Hs6iz3c

Comment 1

[Andrew Swan [:aswan]]

Added a test for the default icon.  For completeness here, perhaps we should also have a test of updating an extension that includes an icon.  Due to the nature of these tests, that means getting extensions signed and landed in the tree, let me know if you think I should add that...

Comment 2

[Andrew Swan [:aswan]]

Attachment: Bug 1335333 Fix icon handling for updates

Review commit: https://reviewboard.mozilla.org/r/109792/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109792/

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 8833566 [details]
Bug 1335333 Fix icon handling for updates

https://reviewboard.mozilla.org/r/109792/#review111156

Yes, we should add a test for an update with an icon

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8833566 [details]
Bug 1335333 Fix icon handling for updates

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109792/diff/1-2/

Comment 5

[Andrew Swan [:aswan]]

(In reply to Dave Townsend [:mossop] from comment #3)
> Yes, we should add a test for an update with an icon

Good think you insisted on that since ... it was broken.
I know you already gave an r+ but do the revisions are big enough, do you mind taking another look?

Comment 6

[Andrew Swan [:aswan]]

Comment on attachment 8833566 [details]
Bug 1335333 Fix icon handling for updates

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109792/diff/2-3/

Comment 7

[Dave Townsend [:mossop]]

Comment on attachment 8833566 [details]
Bug 1335333 Fix icon handling for updates

https://reviewboard.mozilla.org/r/109792/#review111324

::: browser/base/content/test/general/browser_extension_update.js:204
(Diff revisions 1 - 2)
> +
> +add_task(() => backgroundUpdateTest(`${URL_BASE}/browser_webext_update1.xpi`,
> +                                    ID, checkDefaultIcon));
> +
> +function checkNonDefaultIcon(icon) {
> +  dump(`icon is ${icon}\n`);

If you really want to log then use info, otherwise remove this.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3660b57435a3
Fix icon handling for updates r=mossop

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3660b57435a3

Comment 10

[Vasilica Tamas, QA [:vtamas]]

Verified as fixed on Firefox 54.0a1 (2017-02-15) under Windows 10 64-bit and Ubuntu 16.04 32-bit. The add-on puzzle icon is successfully displayed in update permissions doorhanger for those webextensions that have no icon specified in manifest.json.