MatchPattern exception on chrome://favicon/

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
WebExtensions: Untriaged
P1
normal
RESOLVED FIXED
a year ago
11 months ago

People

(Reporter: Andy McKay, Assigned: kmag)

Tracking

unspecified
mozilla56
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox54- unaffected, firefox55+ fixed, firefox56+ fixed)

Details

(Whiteboard: [triaged])

MozReview Requests

()

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

Attachments

(2 attachments)

(Reporter)

Description

a year ago
[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

The extension tree tabs: 

https://addons.mozilla.org/en-US/firefox/addon/tree-tabs/

Won't install in Nightly or Beta. Removing the permission:

"chrome://favicon/"

Let's it install. The exception is:

Exception { message: "", result: 2147942487, name: "NS_ERROR_ILLEGAL_VALUE", filename: "resource://gre/modules/Extension.jsm", lineNumber: 566, columnNumber: 0, data: null, stack: "loadManifest@resource://gre/modules…", location: XPCWrappedNative_NoHelper }
Nightly is 56 and Beta is 55. I can install the addon in 54. Mark 54 back to unaffected.
status-firefox54: affected → unaffected
status-firefox56: --- → affected
tracking-firefox54: ? → -
tracking-firefox56: --- → ?
Comment hidden (mozreview-request)
tracking as breaking a popular extension
tracking-firefox55: ? → +
tracking-firefox56: ? → +
There's my proposal (reviewboard)...take it or leave it.  I think it's fine to continue addon install if a bad/unsupported url is in permissions.  In this case, it's one less papercut.
Attachment #8878581 - Flags: review?(kmaglione+bmo)

Comment 5

a year ago
mozreview-review
Comment on attachment 8878581 [details]
Bug 1373293 ignore and report invalid origin permissions,

https://reviewboard.mozilla.org/r/149902/#review154588
Attachment #8878581 - Flags: review?(aswan) → review+
Attachment #8878581 - Flags: review?(kmaglione+bmo)
Assignee: kmaglione+bmo → mixedpuppy
(Reporter)

Comment 6

a year ago
Should we add a unit test for this?
(Assignee)

Comment 7

a year ago
I'd rather we just change the schema code to drop permissions that aren't valid. We should already be warning about this unknown permission, so adding a check here means we'll wind up warning twice.
(Reporter)

Comment 8

a year ago
Since Shane is out this week, bouncing over to Kris to either land the patch as is, or improve.
Assignee: mixedpuppy → kmaglione+bmo
Priority: -- → P1
Whiteboard: [triaged]

Comment 9

a year ago
I received few emails, asking why my extension cannot install, I was scratching my head for few days already without a clue why and what is going on. Finally I found the exception code which led me here.

Frustrating thing is that it is not showing anything. Just error that extension is corrupt.
I couldn't run it in debug to find out what is wrong, it just does not load.
For now I think you should just ignore invalid permissions, and in the future if you want to block, you should make some warning, like for all other errors. Not just straight "this extensions is corrupt"...

In Chromium, "chrome://favicon/" is needed to access internal and extensions favicons via url.
To access internal favicons, pattern is: "chrome://favicon/" + tab.url.
Without it I have no permission to load favicons from extensions tabs, this is really important for "tabs suspending extensions" that capture original favicon and change opacity to make "unloaded" effect.
Moreover, if I don't add this, console throws billions "Not allowed to load local resource:chrome://xxxxxx".
Anyway this method is used in many extensions like vTabs, Sidewise, Tabs Outliner and so on.
It did not give any errors in Firefox, so I left it there.

Firefox, internal pages still will not allow me to get favicons, so I have to find another way.
Anyone knows how to do that, since I'm not familiar with Firefox APIs? Something like "about" does not work either...
Comment hidden (mozreview-request)

Comment 11

a year ago
mozreview-review
Comment on attachment 8880096 [details]
Bug 1373293: Drop invalid permissions when normalizing manifests.

https://reviewboard.mozilla.org/r/151452/#review156392

Looks good. I just have one question about the test. r+ whether you decide to update the test or not.

::: toolkit/components/extensions/test/xpcshell/test_ext_unknown_permissions.js:20
(Diff revision 1)
> +  await extension.startup();
> +
> +  const {WebExtensionPolicy} = Cu.import("resource://gre/modules/Extension.jsm", {});
> +
> +  let policy = WebExtensionPolicy.getByID(extension.id);
> +  Assert.deepEqual(policy.permissions, ["activeTab", "http://*/*"]);

Do we want to assert that the warning was logged as well? I assume the code added to Schemas.jsm will cause a warning to be logged to the console.
Attachment #8880096 - Flags: review?(bob.silverberg) → review+
(Assignee)

Comment 12

a year ago
mozreview-review-reply
Comment on attachment 8880096 [details]
Bug 1373293: Drop invalid permissions when normalizing manifests.

https://reviewboard.mozilla.org/r/151452/#review156392

> Do we want to assert that the warning was logged as well? I assume the code added to Schemas.jsm will cause a warning to be logged to the console.

Yeah, I suppose I should.
(Assignee)

Comment 13

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b04ed6a0a52af869867351bc0a10e86033e718e8
Bug 1373293: Drop invalid permissions when normalizing manifests. r=bsilverberg

Comment 16

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b04ed6a0a52a
https://hg.mozilla.org/mozilla-central/rev/db257b3d2c99
https://hg.mozilla.org/mozilla-central/rev/d568e7c89e9f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
(Assignee)

Comment 17

a year ago
Comment on attachment 8880096 [details]
Bug 1373293: Drop invalid permissions when normalizing manifests.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1322235
[User impact if declined]: This prevents certain extensions from being installed.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: No, tests should be sufficient.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low-risk.
[Why is the change risky/not risky?]: The change is relatively simple, and mainly just drops permission strings that aren't understood.
[String changes made/needed]: None.
Attachment #8880096 - Flags: approval-mozilla-beta?
Comment on attachment 8880096 [details]
Bug 1373293: Drop invalid permissions when normalizing manifests.

add-on compat fix for 55.0b5
Attachment #8880096 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 19

a year ago
I am now on Nightly Version 56.0a1 (2017-06-23) (64-bit) -- doesn't that mean I should have the fix?
The "tree tabs" extension still doesn't install.
(Assignee)

Comment 20

a year ago
(In reply to Ralf Jung from comment #19)
> I am now on Nightly Version 56.0a1 (2017-06-23) (64-bit) -- doesn't that
> mean I should have the fix?
> The "tree tabs" extension still doesn't install.

Can you post the build source from about:buildconfig? It installs for me in the latest nightly.

Comment 21

a year ago
"Built from https://hg.mozilla.org/mozilla-central/rev/594cc32b632396a867ef1f98428968b224d82151"

The Browser console says
```
1498239786635	addons.xpi	WARN	Download of https://addons.mozilla.org/firefox/downloads/latest/tree-tabs/platform:2/addon-809637-latest.xpi?src=api failed: [Exception... "Illegal value"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource://gre/modules/Extension.jsm :: loadManifest :: line 572"  data: no] Stack trace: loadManifest()@resource://gre/modules/Extension.jsm:572
```
(Assignee)

Comment 22

a year ago
It sounds like you have cached schema data from an older version. I'll bump the cache schema version.

Comment 24

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/b435b2f23945
status-firefox55: affected → fixed
Flags: in-testsuite+

Comment 25

a year ago
> It sounds like you have cached schema data from an older version.

I created this profile yesterday, so it has seen exactly one upgrade -- from yesterday's nightly to today's.

Is there any way for me to clear that cache, to test of this will help?

Comment 26

a year ago
Extension is installing fine on today's nightly.

Comment 27

a year ago
Ralf, I did an update 2 days ago and it was released yesterday. I didn't know when the patch was going to be released. I replaced permissions from "chrome://favicon/" to "<all_urls>".
(Reporter)

Updated

11 months ago
Duplicate of this bug: 1385864
You need to log in before you can comment on or make changes to this bug.