1350053 - Install flow of the new static themes does not match that of LWTs

Status: RESOLVED FIXED

(WebExtensions :: Frontend, defect)

Description

[krupa raj [:krupa--use this to needinfo]]

55.0a1 (2017-03-23) (64-bit)

steps to reproduce:
1. On Nightly, set pref extensions.webextensions.themes.enabled to true
2. Go to https://addons.mozilla.org/en-US/firefox/themes/?sort=popular and install any of the old-style LWTs
3. Notice that after the install is successful there is a notification bar at the top with options to Undo/Manage Themes.
4. Install a new static theme and notice the install flow

expected behavior:
install flow for the new static themes is similar to old-style LWTs

actual behavior:
install flow for the new static themes is exactly like a webextension which may not be apt for themes.

screencast:
https://www.dropbox.com/s/242mgg5nlulz8y1/installflow.mov?dl=0

Comment 1

[Mike de Boer [:mikedeboer]]

Krupa, how did you create that .xpi? Can you tell me how you created it or point me to a wiki page? Thanks!

Comment 2

[krupa raj [:krupa--use this to needinfo]]

Attachment: all_the_elements-2.0-an+fx.xpi

Steps to create a new-style static theme:
1. Create a manifest.json using the sample listed in https://docs.google.com/document/d/1ueD6V7aLLTuc1GAOxxQYcwl2HR-k62HHu3q8knTJ4FU/edit?pli=1#heading=h.bij4y75jj419
2. Link to the relevant images in the folder which is zipped and then converted to an xpi (header image was copied from an existing LWT)
3. Have the .xpi file signed by uploading it as an unlisted add-on (you can skip this step on Nightly by turning the signing off)

Let me know if these steps are clear enough.

Comment 3

[Mike de Boer [:mikedeboer]]

Attachment: WIP Patch: Change install flow of LWTs to use PopupNotifications

Dave, before I update all the tests, what do you think of this approach?

I wanted LWTs to adopt the WebExtensions Themes flow, because retro-fitting felt like a bad idea and unnecessary maintenance burden.

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8855812 [details] [diff] [review]
WIP Patch: Change install flow of LWTs to use PopupNotifications

Review of attachment 8855812 [details] [diff] [review]:
-----------------------------------------------------------------

The approach seems fine however I think you'll need to rev the l10n ids since you're displaying them in a different context.

Has UX signed off on this change though?

Comment 5

[Mike de Boer [:mikedeboer]]

Comment on attachment 8855812 [details] [diff] [review]
WIP Patch: Change install flow of LWTs to use PopupNotifications

Hi Markus, can you review this change by watching the following two videos:

 - Status Quo - https://vimeo.com/212581150
 - Now using doorhangers - https://vimeo.com/212581162

?

Please let me know if you have any further questions! (I requested a ScreenFlow license again, so next time they will be more pro-quality ;-) )

Comment 6

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a447797110ad

Comment 7

[Markus Jaritz [:designakt] (UX)]

Comment on attachment 8855812 [details] [diff] [review]
WIP Patch: Change install flow of LWTs to use PopupNotifications

Looks great! I love that you got rid of the notification bar! Thank you.

Comment 8

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1350053 - Part 1 - Change install flow of Light-weight Themes (LWT) to use PopupNotifications, instead of notification boxes.

The newer Theming API follows the WebExtension install flow, which uses
PopupNotifications to request permission before installation, show progress and
notify upon install completion. This patch makes sure that older LWTs follow that
same flow.

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

Comment 9

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1350053 - Part 2 - Update tests to check the arrow panel to appear, instead of the notification bar.

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

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 8856591 [details]
Bug 1350053 - Part 1 - Change install flow of Light-weight Themes (LWT) to use PopupNotifications, instead of notification boxes.

https://reviewboard.mozilla.org/r/128546/#review130994

Comment 11

[Dave Townsend [:mossop]]

Comment on attachment 8856592 [details]
Bug 1350053 - Part 2 - Update tests to check the arrow panel to appear, instead of the notification bar.

https://reviewboard.mozilla.org/r/128548/#review130996

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c0565ed332f
Part 1 - Change install flow of Light-weight Themes (LWT) to use PopupNotifications, instead of notification boxes. r=mossop
https://hg.mozilla.org/integration/autoland/rev/74d44ad1533e
Part 2 - Update tests to check the arrow panel to appear, instead of the notification bar. r=mossop

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5c0565ed332f
https://hg.mozilla.org/mozilla-central/rev/74d44ad1533e