1365568 - chrome is not registered for non-bootstrapped proxy file add-ons

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Denis Lisov]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170517100341

Steps to reproduce:

I'm developing Brief preparing it for WebExtensions migration. In the extensions directory there is a file named after the addon id containing the path to the git checkout used for development. A chrome package is registered with a line in chrome.manifest

content  brief  chrome/content/ remoteenabled=yes remoterequired=yes


Actual results:

In Firefox Nightly 2017-05-16 and 2017-05-17 the addon is no longer working. The add-on manager and about:support both show it as enabled, but no UI overlay is displayed and trying to navigate to the add-on page ( chrome://brief/content/brief.xhtml ) results in "No chrome package registered for chrome://brief/content/brief.xhtml" in the browser console.

I've narrowed down the regression window with mozregression to https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9730f88bdbd3d167d3c5b1acb0618cd2533bb570&tochange=23dc7b443bb0528ec880acb7fc0a32aec05f0255

Comment 1

[Loic]

You are hitting bug 1365256 which has been fixed this night. Are you sure your Nightly is up-to-date (2017-05-17)?

I'm able to open and display the page chrome://brief/content/brief.xhtml and I don't have any error message in the console.

Could you test again, please.

Comment 2

[Denis Lisov]

The About window shows 2017-05-17. Reproduced successfully.

Reproduction steps:

git clone https://github.com/ancestorak/brief/ # development repository

# Start Nightly, create new profile, close Nightly

cd $PROFILE_DIR
mkdir extensions
echo "/path/to/cloned/brief/" > extensions/[e-mail]

# Start nightly
# about:config - switch xpinstall.signatures.required to false
# about:addons - enable Brief, "restart now"

# Now, after restart, Brief is enabled according to about:addons and about:support
# However, there's no Brief button and the URL chrome://brief/content/brief.xhtml does not work


Is the path-based installation for development not supported now?

Comment 4

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

[Tracking Requested - why for this release]: Regression

Comment 6

[Kris Maglione [:kmag]]

Attachment: Bug 1365568: Enable chrome registration for non-bootstrapped proxy file add-ons.

Paths for proxy file add-ons are not relative to their install location, so
they require special treatment when resolving the full paths to their
manifests. Additionally, on Windows, absolute paths are also technically valid
as relative paths, so we need to try parsing them as absolute paths before
falling back to relative paths, rather than the reverse.

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

Comment 7

[Robert Helmer [:rhelmer]]

Comment on attachment 8869784 [details]
Bug 1365568: Enable chrome registration for non-bootstrapped proxy file add-ons.

https://reviewboard.mozilla.org/r/141338/#review144902

r+ assuming there's a good reason not to have a more explict exception for the `catch`... otherwise please fix and land no need for re-review.

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:133
(Diff revision 1)
> -    let file = base.clone();
> +  // files. Ideally we would try a relative path first, but on Windows,
> +  // paths which begin with a drive letter are valid as relative paths,
> +  // and treated as such.
> -    try {
> +  try {
> +    return new nsIFile(path);
> +  } catch (e) {

I'm not a big fan of catch-all exceptions... what does this throw, `NS_ERROR_FILE_UNRECOGNIZED_PATH`?

Ideally we should only catch the one we care about, especially if we're going to silently swallow it.

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8869784 [details]
Bug 1365568: Enable chrome registration for non-bootstrapped proxy file add-ons.

https://reviewboard.mozilla.org/r/141338/#review144902

> I'm not a big fan of catch-all exceptions... what does this throw, `NS_ERROR_FILE_UNRECOGNIZED_PATH`?
> 
> Ideally we should only catch the one we care about, especially if we're going to silently swallow it.

Yes, something like that. But the only things this will ever throw for are invalid paths and OOM, and in either case a failure that we need to care about will be caught by the relative path version below, so there's not much point in doing more specific checks.

Comment 9

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d2fc6ed0d64123096581a75590bb1582efc182fc
Bug 1365568: Enable chrome registration for non-bootstrapped proxy file add-ons. r=rhelmer

Comment 10

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/d2fc6ed0d641

Comment 11

[Jonathan Kamens]

Confirmed: add-ons installed via proxy files in user's profile/extensions directory are now loading properly in Thunderbird Daily.