1486416 - In the sources list, extension entries should have their name instead of their uuid

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P3)

Description

[Julien Wajsberg [:julienw]]

Attachment: screenshot

This is a follow-up to the github issue [1].

Here is the STR:

1. Install React Addon from https://addons.mozilla.org/en-US/firefox/addon/react-devtools/
2. Install UBlock Origin from https://addons.mozilla.org/fr/firefox/addon/ublock-origin/
3. Go to any page, for example www.mozilla.org
4. Open the debugger
5. We have 2 entries for the 2 extensions, but we only see their internal URLs containing the UUIDs. For a developer, this is near useless as there's no easy mean to look-up an extension from their UUIDs.

(See attachment)

[1] https://github.com/devtools-html/debugger.html/issues/5815

Julian Descottes commented there:

Not sure where the server is getting the addon uuid, but you could use the WebExtensionPolicy to convert from moz-extension:// to the addon name:

    const uri = "moz-extension://4ade278d-b4ba-1c45-9398-7d33741c5aa6/";
    const name = WebExtensionPolicy.getByURI(Services.io.newURI(uri)).name;

This is just from a quick search in our codebase, @rpl might know a better way.
But the source actor already seems to be doing some logic around this in _mapSourceToAddon (check how it is using mapURIToAddonID).

Comment 2

[:Harald Kirschner :digitarald]

ni? Luca to provide input on how this could be solved; as bugs keep coming in and this makes debugging a lot of intuitive.

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to :Harald Kirschner :digitarald from comment #2)

ni? Luca to provide input on how this could be solved; as bugs keep coming in and this makes debugging a lot of intuitive.

Sure, absolutely!

On the "Firefox Remote Debugging Server"-side, as :julienw mentioned in comment 0, the method SourceActor's _mapSourceToAddon method is already doing some work related to the extensions, in particular it retrieves the addonId from the extension url, and so we should adjust the SourceActor to also:

• in _mapSourceToAddon, retrieve the addon name, e.g. by calling directly WebExtensionPolicy.getByURI(Services.io.newURI(uri)).name;, or from an helper module like "map-uri-to-addon-id.js" (we could even make both part of the same helper method/module, so that we can retrieve both the addon id and the addon name from the same single call),
  and then assigning it to this._addonName (e.g. near here: https://searchfox.org/mozilla-central/rev/7adb490485eff0783071a3e132005bceeb337461/devtools/server/actors/source.js#257-261)

• in the SourceActor's form method, add the addonName to the properties returned to the RDP client

Once the SourceActor is able to retrieve and send to the RDP client the addonName as part of the SourceActor form, then the only missing piece of the puzzle should be on the "Debugger client UI" side:

• make the debugger client UI internals aware of this new addonName property (e.g. I think that we have to add them to the related flow types, e.g. SourcePayload)

• change the related React components (and likely some related Redux actions/reducers) of the Debugger UI to render the addonName when available (as it could be missing if the debugger client is connected to a version of the RDP server that doesn't provide the addonName property yet), and we could may be turn the full "moz-extension://" url into a tooltip of that entry in the outline when the it is using the addon name

Comment 5

[David Walsh :davidwalsh]

Attachment: Bug 1486416: Show addon name in debugger sources tree

This is a WIP

Comment 6

[David Walsh :davidwalsh]

Hello :rpl! I'm super excited to implement this!

I took some time today to look at your suggested implementation. The addon logic was removed in (https://hg.mozilla.org/mozilla-central/rev/bfdfbd4fa2cea68683bb2ad792aeefa8499475af); I backed out that change locally, then implemented your suggestion, but I think there's one piece of logic I need help with. The resolveURIToLocalPath call is always returning null because we don't have a case for moz-extension, and I don't know how to what service/utillty to use to get the usable URI which would allow the WebExtensionPolicy.getByURI(Services.io.newURI(uri)).name; to succeed.

I started a WIP patch; when you get time, let me know if I'm missing something simple or more is needed to accommodate for moz-extension. Thank you!

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to David Walsh :davidwalsh from comment #6)

Hello :rpl! I'm super excited to implement this!

Hi David, I'm so happy that you are taking a look into fixing this!

I took some time today to look at your suggested implementation. The addon logic was removed in (https://hg.mozilla.org/mozilla-central/rev/bfdfbd4fa2cea68683bb2ad792aeefa8499475af); I backed out that change locally, then implemented your suggestion, but I think there's one piece of logic I need help with. The resolveURIToLocalPath call is always returning null because we don't have a case for moz-extension, and I don't know how to what service/utillty to use to get the usable URI which would allow the WebExtensionPolicy.getByURI(Services.io.newURI(uri)).name; to succeed.

I started a WIP patch; when you get time, let me know if I'm missing something simple or more is needed to accommodate for moz-extension. Thank you!

Ok, I took a quick look to the code removed in Bug 1523942, and I think that we can aim to do something way simpler now that we don't have support for the legacy extensions anymore, I'm pretty sure that the old _mapSourceToAddon was that complex and it was using resolveURIToLocalPath to resolve the uri into a path because the legacy extensions were blending themselves into the Firefox resource and chrome urls, and they did not have a separate moz-extension:// schema for the extensions urls as the WebExtensions do.

And so I think that it should be now possible to just do something simpler like the following:

• Retrieving the source's url using SourceActor url getter into an URI (e.g. using Services.io.newURI)

• Then trying to retrieve an extension policy instance for that URI using WebExtensionPolicy.getByURI and, if the policy object exists (which means that the extension in installed, enabled and running) then retrieves the extension name and id from it and add it to the SourceActor's form

How that sounds to you?

Let me know what you think about it and how it goes once you give it a try (I'll also keep an eye to the updates applied on the draft patch attached to this issue).

Comment 8

[David Walsh :davidwalsh]

Attachment: Bug 1486416 - Show extension name in the debugger r=jlast,rpl

Showing moz-extension URLs in the debugger is not helpful -- we should display the name of the extension instead.

Comment 9

[David Walsh :davidwalsh]

@rpl: I've implemented this and it works great except I don't get a property name from the policy, I get debugName which includes the plugin name, ID, and other information (i.e. the screenshot in my patch). Is there a singular string I can get somehow that is just the extension name?

Comment 10

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to David Walsh :davidwalsh from comment #9)

@rpl: I've implemented this and it works great except I don't get a property name from the policy, I get debugName which includes the plugin name, ID, and other information (i.e. the screenshot in my patch). Is there a singular string I can get somehow that is just the extension name?

yep, we actually want policy.name instead of debugName.

Comment 11

[David Walsh :davidwalsh]

Ugh, sorry :rpl! :(

When looking at the console output, I saw:

({debugName:"\"Pinterest Save Button\" (ID: jid1-YcMV6ngYmQRA2w@jetpack, moz-extension://a657228a-e4e3-7e45-ba22-e0bde4801862/)", instanceId:8, optionalPermissions:[]})

You're right, .name is there but doesn't show in the console.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5598fb784329
Show extension name in the debugger r=jlast,rpl,jdescottes

Comment 13

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/5598fb784329

Comment 14

[:Harald Kirschner :digitarald]

Tagging as this affordance is release-note-worthy