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

RESOLVED FIXED in Firefox 69

Status

enhancement
P3
normal
RESOLVED FIXED
Last year
14 days ago

People

(Reporter: julienw, Assigned: davidwalsh)

Tracking

(Blocks 2 bugs, {dev-doc-needed})

unspecified
Firefox 69
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox69 fixed)

Details

(Whiteboard: [debugger-mvp][devtools-backward-compat])

Attachments

(2 attachments, 1 obsolete attachment)

Posted image 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).
Severity: normal → enhancement
Priority: -- → P3
Duplicate of this bug: 1474544

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

Flags: needinfo?(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:

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

Flags: needinfo?(lgreco)
Duplicate of this bug: 1519288

This is a WIP

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!

Flags: needinfo?(lgreco)
Assignee: nobody → dwalsh
Status: NEW → ASSIGNED
Whiteboard: [debugger-mvp]

(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).

Flags: needinfo?(lgreco)
Attachment #9070707 - Attachment is obsolete: true

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

@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?

Flags: needinfo?(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.

Flags: needinfo?(lgreco)

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.

Whiteboard: [debugger-mvp] → [debugger-mvp][devtools-backward-compat]
Pushed by dwalsh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5598fb784329
Show extension name in the debugger r=jlast,rpl,jdescottes
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 69

Tagging as this affordance is release-note-worthy

Keywords: dev-doc-needed
Blocks: 1565711
Blocks: 1565713
No longer blocks: 1565711
No longer blocks: 1565713
You need to log in before you can comment on or make changes to this bug.