Closed
Bug 1371752
Opened 8 years ago
Closed 8 years ago
Legacy extension includes an unsigned extension
Categories
(Toolkit :: Add-ons Manager, defect, P2)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: andy+bugzilla, Assigned: aswan)
References
Details
(Whiteboard: triaged)
Attachments
(2 files)
On Nightly
1. turn off extension signing requirement
2. turn on legacy add-on restrictions
3. install an unsigned add-on.
The add-on my add-ons list, it also appears in the legacy extensions list. Because that page says at the top "... we deactivated them" that's confusing.
Perhaps we can leave add-ons off that page if the extension signing requirement is turned off.
This will never happen in release, so prioritize accordingly.
Reporter | ||
Comment 1•8 years ago
|
||
This will only occur if you have at least one legacy extension.
Updated•8 years ago
|
Assignee: nobody → aswan
Priority: -- → P2
Whiteboard: triaged
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8877318 -
Flags: review?(rhelmer)
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8877318 [details]
Bug 1371752 Don't show unsigned extensions in legacy pane if signatures are disabled
https://reviewboard.mozilla.org/r/148656/#review153572
I'm not sure I really agree with the premise of this bug... a legacy extension is a legacy extension, regardless of whether signing is enabled on the build, right? I don't feel strongly enough to r- for that reason alone though.
::: toolkit/mozapps/extensions/content/extensions.js:320
(Diff revision 1)
> // Add-ons without an "isCorrectlySigned" property are correctly signed as
> // they aren't the correct type for signing.
> return aAddon.isCorrectlySigned !== false;
> }
>
> +function isDisabledUnsigned(addon) {
This function name is confusing to me, I read it as "is this add-on both disabled and unsigned".
What do you think? Would something like `isSigningDisabled(addon)` be clearer?
Attachment #8877318 -
Flags: review?(rhelmer) → review+
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Robert Helmer [:rhelmer] from comment #4)
> I'm not sure I really agree with the premise of this bug... a legacy
> extension is a legacy extension, regardless of whether signing is enabled on
> the build, right? I don't feel strongly enough to r- for that reason alone
> though.
Well we still put a warning on individual unsigned extensions. The point here is that the UX is meant to separate extensions that you are using (or can use but have manually disabled) from those that you just can't use at all. With usable extensions in the Extensions pane and unusable ones in the "legacy" pane. In the specific case of Nightly with the signatures pref flipped, unsigned webextensions are usable so they shouldn't appear in the "unusable" list.
> > +function isDisabledUnsigned(addon) {
>
> This function name is confusing to me, I read it as "is this add-on both
> disabled and unsigned".
>
> What do you think? Would something like `isSigningDisabled(addon)` be
> clearer?
Oh, it was mean to be a more concise version of "this is disabled because it is unsigned". I think isSigningDisabled is confusing, it sounds to me like it is about the global setting, not about an individual extension.
Comment 6•8 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #5)
> (In reply to Robert Helmer [:rhelmer] from comment #4)
> > > +function isDisabledUnsigned(addon) {
> >
> > This function name is confusing to me, I read it as "is this add-on both
> > disabled and unsigned".
> >
> > What do you think? Would something like `isSigningDisabled(addon)` be
> > clearer?
>
> Oh, it was mean to be a more concise version of "this is disabled because it
> is unsigned". I think isSigningDisabled is confusing, it sounds to me like
> it is about the global setting, not about an individual extension.
Fair enough... since it takes addon as an arg I was hoping it'd read like "is signing disabled for this add-on" but that is confusing too.
The core issue I have is that "disabled" makes me think of the add-on state (user/app disabled etc)
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Robert Helmer [:rhelmer] from comment #6)
> The core issue I have is that "disabled" makes me think of the add-on state
> (user/app disabled etc)
Well it is certainly meant to mirror that. If this function returns true, then the addon.appDisabled should also be true. Of course, the addon could be appDisabled for some other reason as well so the reverse isn't true.
Pushed by aswan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6166e9796a59
Don't show unsigned extensions in legacy pane if signatures are disabled r=rhelmer
![]() |
||
Comment 9•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•