1488500 - [remote-dbg-next] Show warning when temporary install fails

Status: RESOLVED FIXED

(DevTools :: about:debugging, enhancement, P1)

Description

[Julian Descottes [:jdescottes]]

Current about:debugging displays several warning when a temporary addon fails to install. This feature should be ported to the new about:debugging.

See usage of localized strings:
- addonInstallError
- legacyExtensionWarning

Comment 1

[Emily Toop (:fluffyemily)]

Hi. I was pointed at this as a good first bug (first time in devtools land, and working on Mozilla Central). Is there anything that I should know about where/how I should go to implement this?

I can see that the strings you've highlighted are in `devtools/client/aboutdebugging/components/addons/Target.js` & `devtools/client/aboutdebugging/components/addons/InstallError.js`. It looks like this issue is about moving the error functionality from those files into `devtools/client/aboutdebugging-new/components`. Does the whole addons functionality need porting too or just that error capability? 

How do I go about using the new about:debugging opposed to the original one?

What are the best STR for testing this is working (i.e. how would I install a temporary addon that fails)?

Thanks,

Emily

Comment 2

[Julian Descottes [:jdescottes]]

Thanks Emily! 

To setup your m-c environment we have some documentation at https://docs.firefox-dev.tools/getting-started/build.html

In order to use the new about:debugging, you need to set the preference `devtools.aboutdebugging.new-enabled` to true in about:config. This new version already supports installing temporary addons, but never displays any error message if the install fails.

The steps here are:
- open about:debugging
- click on Load Temporary Addon
- find the path to an invalid extension (we have one in the mozilla central repo, under devtools/client/aboutdebugging/test/addons/bad/manifest.json so you can just navigate to this json and try to load it)

Expected Result: the installation error should be displayed to the user 
Actual Result: nothing happens (you can try this on the current about:debugging to see how the feature looks like today) 

For the UI, we can reuse the same colors and icons as the ones used in the current about:debugging. 

The error message also contains a "Retry" button, to retry installing the addon at the same path. 

In this bug let's keep things simple and only implement a "Dismiss" button that will hide the message. We will implement the Retry feature in a follow-up bug.

Comment 3

[Emily Toop (:fluffyemily)]

Hi. I realise I shouldn't have picked this up and then headed off on PTO. Sorry about that.

I am trying to figure out where is best to handle the extenstion installation failure mode. Can I please query about the `runtimeReducer` (`runtime-state.js`)? I notice that in all of the `actionTypes` defined in `constants.js`, only the success conditions are implemented in `runtimeReducer`. Is this deliberate and failure states should be implemented somewhere else, or is the project currently still a in happy-path only state (I can't find any failure state implementations anywhere)?

Comment 4

[Emily Toop (:fluffyemily)]

Actually, ignore that, I think I just figured it out. I think I need to examine the details of the props target as shown in `ServiceWorkerAction` and render different things based on the output.

Comment 5

[Soledad Penades [:sole] [:spenades]]

Emily told me she's no longer working on this, so unassigning

Comment 6

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 7

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 8

[Julian Descottes [:jdescottes]]

filter on remote-debugging-next-move-m3-to-m2

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: existing_mockups.png

Matt, I plan to do a simple implementation to show install errors for temporary addons. I will follow the original mockups for now. Would it make sense to have a UX followup for this particular item, or for installing temporary addons in general?

Comment 10

[Julian Descottes [:jdescottes]]

Actually I can't really follow the mockups here because when a temporary installation fails, we don't have the necessary metadata about the addon to display its name, id, icon etc...

Comment 11

[Julian Descottes [:jdescottes]]

Attachment: error_message_example_aboutaddons.png

Comment 12

[Julian Descottes [:jdescottes]]

Attachment: Bug 1488500 - Show install error message for temporary addons;r=daisuke,ladybenko

Comment 13

[Julian Descottes [:jdescottes]]

Attachment: Bug 1488500 - Add test for temporary extension install error;r=daisuke

Depends on D12571

Comment 14

[mcroud]

Yes a UX bug for this would be good.

Can I confirm my understanding that the use case for this feature is to allow developers to install and run an unsigned add on they are developing?

Is there a way I can test this failure myself?

Comment 15

[Julian Descottes [:jdescottes]]

(In reply to Matthew Croud from comment #14)
> Yes a UX bug for this would be good.
> 
> Can I confirm my understanding that the use case for this feature is to
> allow developers to install and run an unsigned add on they are developing?
> 

Yes, that's exactly the idea. Filed Bug 1509091 and wrote some more details about the feature in https://docs.google.com/document/d/1GKOODQaMyO9eMu0vgXQybHO2HFR_pL3z9JGzF5c25qE/edit# 

> Is there a way I can test this failure myself?

You can have a look at the (slightly old) documentation at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Temporary_Installation_in_Firefox

The easiest is probably to clone the source code for an existing extension (eg https://github.com/captainbrosset/devtools-highlighter) and try to load it as a temporary extension, modify a few things, reload etc... 

Note that there is an alternate workflow for developing addons which relies on webext https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Getting_started_with_web-ext . I don't know if one is more popular than another, I don't think we have telemetry for those.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11412ab473bc
Show install error message for temporary addons;r=daisuke,ladybenko
https://hg.mozilla.org/integration/autoland/rev/8b2231ca82cd
Add test for temporary extension install error;r=daisuke

Comment 17

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/11412ab473bc
https://hg.mozilla.org/mozilla-central/rev/8b2231ca82cd