Show the filesystem location of the extension

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
WebExtensions: Developer Tools
P5
normal
RESOLVED FIXED
5 months ago
2 months ago

People

(Reporter: andym, Assigned: mstriemer)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: triaged)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

5 months ago
This is hopefully a simple change to about:debugging to show the path to the extension on the file system. Particularly useful for temporary extensions, but could apply to them all.
(Reporter)

Updated

5 months ago
Assignee: nobody → mstriemer
Priority: -- → P5
Whiteboard: triaged
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 3

2 months ago
Created attachment 8859706 [details]
install-location.png

This only shows the install location for temporary add-ons. When an add-on is regularly installed from the filesystem the path is set to the original install location which doesn't seem very helpful.

We could use `_sourceBundle` to get the location for regular installs but this ends up being in Firefox's directory for system add-ons and likely also isn't very helpful (and looks kind of bad because those paths are really long and weird).

There's some RTL/LTR flipping for getting the ellipsis on the left-hand-side of the file path. I think this is fine because we always want the file path to be displayed LTR from what I can tell.
(Assignee)

Comment 4

2 months ago
I wanted to add some more specific tests for this by rendering the AddonTarget component directly. This didn't seem to work in the browser tests since there's no global `document` and React depends on that.

I tried writing a mochitest that does this but I couldn't get it to load react. I think this might just need some config updates but I trial and error did not lead me to a solution.

Is there a good way to do this? I don't see any tests like this in aboutdebugging but I see that there are some other parts using enzyme.

Essentially I want this:

    ReactDOM.render(
      React.createElement(AddonTarget, { target: { url: "https://addons.mozilla.org/..." } }, null),
      testContainer);
    ok(!testContainer.textContent.includes("Location"), "remote location is hidden");

Comment 5

2 months ago
mozreview-review
Comment on attachment 8859693 [details]
Bug 1330741 - Show filesystem location of add-ons in about:debugging

https://reviewboard.mozilla.org/r/130884/#review135238

Looks good to me, thanks! 
Few nits, feel free to land with my comments addressed and a green try.

::: devtools/client/aboutdebugging/aboutdebugging.css:258
(Diff revision 2)
> +  display: grid;
> +  font-size: 14px;
> +  grid-template-columns: 128px 1fr;
> +}
> +
> +.addon-target-info dt {

can you use classes instead of dt/dd ?

::: devtools/client/aboutdebugging/test/browser.ini:31
(Diff revision 2)
>  tags = webextensions
>  [browser_addons_debug_webextension_nobg.js]
>  tags = webextensions
>  [browser_addons_debug_webextension_popup.js]
>  tags = webextensions
> +[browser_addons_debug_info.js]

nit: move before [browser_addons_debug_webextension.js]

::: devtools/client/aboutdebugging/test/browser_addons_debug_info.js:21
(Diff revision 2)
> +  let container = document.querySelector(`[data-addon-id="${ADDON_ID}"]`);
> +  let filePath = container.querySelector(".file-path");
> +  let expectedFilePath = "browser/devtools/client/aboutdebugging/test/addons/unpacked/";
> +
> +  // Verify that the path to the install location is shown next to its label.
> +  ok(filePath, "no file path set");

The description seems wrong here?
Attachment #8859693 - Flags: review?(jdescottes) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 7

2 months ago
Try build [1] look good. Marking checkin-needed.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3a92c0f8292031c90a39696d50b55e53ec4e5a1
Keywords: checkin-needed
Autoland can't push this until all pending issues in MozReview are marked as resolved.
Keywords: checkin-needed
(Assignee)

Updated

2 months ago
Keywords: checkin-needed

Comment 9

2 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3974fea2cf2d
Show filesystem location of add-ons in about:debugging r=jdescottes
Keywords: checkin-needed

Comment 10

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3974fea2cf2d
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.