Closed Bug 476430 Opened 15 years ago Closed 13 years ago

Make third-party add-ons disabled on startup, and allow users to activate them

Categories

(Toolkit :: Add-ons Manager, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla8
Tracking Status
blocking2.0 --- -

People

(Reporter: jetxee, Assigned: mossop)

References

(Depends on 2 open bugs, Blocks 1 open bug, )

Details

(Keywords: uiwanted, ux-control, Whiteboard: [sg:want])

Attachments

(1 file, 4 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.9.0.5) Gecko/2008121622 Ubuntu/8.10 (intrepid) Firefox/3.0.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.9.0.5) Gecko/2008121622 Ubuntu/8.10 (intrepid) Firefox/3.0.5

I think only those add-ons which the user explicitly enabled in the add-on manager (or installed himself), may be activated. Any add-ons installed system-wide or add-ons which are shipped with third party applications should be disabled by default.

Reproducible: Always

Steps to Reproduce:
1. Install FF on Windows
2. Install .NET 3.5 SP1
3. 
Actual Results:  
FFClickOnce add-on is installed automatically, which is impossible to uninstall normally (http://en.wikipedia.org/wiki/ClickOnce#Firefox_extensions). The add-on is found and enabled automatically.

Expected Results:  
FFClickOnce (or any other 3rd party add-on) should have remained disabled unless the user enabled it explicitely in the add-on manager.

This should apply also to Linux, where system-wide add-ons may be installed by unrelated packages.
I guess this is a wontfix.
Component: Phishing Protection → Add-ons Manager
Product: Firefox → Toolkit
QA Contact: phishing.protection → add-ons.manager
I'm going to morph this bug because certainly disabling them by default is not the way we would go. However there is potential in displaying to the user the new third party add-ons installed on next startup and giving them the option to disable at that time. It needs more discussion before we'd go ahead and take this but I expect we need a bug to cover it for all the duplicates that are bound to get filed.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Disable any third-party add-on unless explicitly allowed by the user → Warn user that a third party add-on has been installed and allow disabling
Version: unspecified → Trunk
Well, I agree. A warning with an option to disable is OK. Silently enabling whatever code that has been dropped to add-ons' folder is wrong.
There _is_ already a warning but it's IMO to hard to notice.
I don't think I'd say it is wrong as such. It is wrong for third party installers to install extensions without telling you certainly, and wherever that is happening we should be reaching out to those third parties to improve the situation. But that isn't the case here.
(In reply to comment #4)
> There _is_ already a warning but it's IMO to hard to notice.

If you mean that we display the add-ons manager after new add-ons have been installed then potential improvements to that are being covered in bug 447570
(In reply to comment #1)
> I guess this is a wontfix.

I suggest to think about it once more. Just imagine that MS decides to change something which is completely against the way Mozilla intended it to be, potentially modifying the way Firefox works for 80-90% of the user base. I'd vote for disabling by default if it was dropped into the add-ons folder without the consent of the user.
(In reply to comment #7)
> (In reply to comment #1)
> > I guess this is a wontfix.
> 
> I suggest to think about it once more. Just imagine that MS decides to change
> something which is completely against the way Mozilla intended it to be,
> potentially modifying the way Firefox works for 80-90% of the user base. I'd
> vote for disabling by default if it was dropped into the add-ons folder without
> the consent of the user.

I think the key sentence there is "if it was dropped into the add-ons folder without the consent of the user". In that circumstance I would be more inclined to look to the blocklist to disable something that impacts Firefox so much and is getting installed without warning. But again, that isn't this case, I've been told that the MS installer does indicate that it is installing a Firefox add-on, though please correct me if I'm mistaken I have yet to verify for myself.
I don't know since I was alerted like most others by the reports out there. It might be exaggerated, but actually this could have been  and can be any malware or adware as well anytime. I expected that Firefox would have a decent response already in place, but apparently this isn't the case. This clearly needs fixing IMO.
If malware runs on the system it can also override a default setting and enable the addon by modifying Firefox files. As mossop already wrote, the installer of such addons must notify the users that they install a Firefox extension and every software I've encountered did that.

The Firefox notification should be improved but that's bug 447570
(In reply to comment #9)
> I don't know since I was alerted like most others by the reports out there. It
> might be exaggerated, but actually this could have been  and can be any malware
> or adware as well anytime. I expected that Firefox would have a decent response
> already in place, but apparently this isn't the case. This clearly needs fixing
> IMO.

Eddy, what response do you imagine we'd have to actual malware that already has privilege on the system? As Matti says, any such malware could disable that protection, or replace Firefox wholesale while rewriting the operating system. I don't think we'll find much that we can do to be helpful in such a situation.

I think what this bug is really about, and what Dave is responding to, is that there is a class of nuisance software which is NOT malware in the traditional sense, but which nevertheless might act with different aims than our users, and we should make sure they're aware of it so that they can take steps to fix it or complain to the company that produced it.  But that's where the conversation should stay - non-malicious-but-surprising extensions/plugins.
I agree, Johnath....just got carried away a little ;-)
(In reply to comment #8)
> I've
> been told that the MS installer does indicate that it is installing a Firefox
> add-on, though please correct me if I'm mistaken I have yet to verify for
> myself.

There is no warning when it's installed as part of .NET 3.5 SP1, which is probably installed for most users via Windows Update. The associated KB article (http://support.microsoft.com/kb/951847) doesn't mention it. The only brief mention is here (http://windowsclient.net/wpf/wpf35/wpf-whats-new-35sp1.aspx), which I found via links in a forum.

It's also installed via some Visual Studio update or upgrade last summer; I don't know if there is a warning there.

OT: The extension is documented here: http://msdn.microsoft.com/en-us/library/cc716877.aspx
I'm of the strong opinion that users should be prompted to enable these add-ons when they next start Firefox and that until the user does enable them, they should be disabled. Implementing that should be relatively cheap (easy for me to say).

My main concern is the damage the add-on can do before the user disables it. As someone wrote in bug 446139 (in a slightly different context):

> While the add-on is enabled though it is able to make changes
> that can still be in effect after the add-on is disabled. It
> could set cookies for example that continue to be sent, it could
> also change core preferences that affect how Firefox operates.


Under the current setup, by the time the user disables it, it's too late. I just disabled ffclickonce when it first loaded, but my user-agent string was still sending the info appended by the extension. There was no way for me to prevent it. That's a minor issue, but there's no reason the next add-on won't break something serious, if not Firefox itself then maybe my favorite extension.

The only mitigation of that threat is the user's vigilance when running other installers. I don't think that's realistic: 1) The software vendor does not always warn the user or even document it, not even Microsoft (see comment #13); 2) Users don't read documentation before installing. We can say they should, but they won't. Nobody has time, few users could understand it, and nobody does it. I certainly don't read it all, and I'll bet you don't either. (Telling users to blame the vendor of the installer won't improve the situation either.)

I propose that when an add-on is not explicitly installed by the user via Firefox' UI:
1) It should not be enabled until the user explicitly enables it.
2) The following dialog greet the user when appropriate (probably the next time Firefox starts):

    ---------------------------------------------------------------
    Add-on <NAME> was installed by the program <INSTALLERNAME> at
    <TIME>, <DATE>.  Do you want to enable it?

    Add-ons can add helpful features to Firefox. Because they
    make changes to Firefox, they can also cause Firefox to
    malfunction. Only enable Add-ons from trusted sources.

    Usually, these add-ons are installed during the installation
    of other programs on your computer. For more information about
    this add-on, read its documentation or contact the vendor of
    the program that installed it.

    [Enable it]  [Do NOT enable it (I can enable it later)]
    ---------------------------------------------------------------

3) Store info in the first paragraph (installer name, date & time) someplace in the Add-ons interface. It's necessary for the user to identify it. When I discovered ffclickonce on my machine, it took me a good amount of research to discover its source. I have some plugins for which I have no idea of the provenance.
>My main concern is the damage the add-on can do before the user disables it
That is no reason at all. The software that installs an addon runs already on your system and can do everything it wants. Sending your private browser history to the google datacollectors or send your passwords file to anyone else or install a keylogger.
There is a list in Firefox if an addon is enabled or not and the installer could just modify the firefox files that the addon should be enabled or that it's installed a long time before.
This will happen if you disable it per default and i think that's enough to not do this because we don't want other software modify our files and breaking it.
You already confirmed that you want this addon if you started the installer.exe on your system which installs the software on your system and the addon, you only don't know it.

> Add-on <NAME> was installed by the program <INSTALLERNAME> at <TIME>, <DATE>.  >Do you want to enable it?

Installername and date/time are unknown.
Matti,

I think the scenario you are considering is some sort of attack, like malware. I'm just talking about legit software, which will presumably not try to bypass this configuration.

I agree that it is of limited help with malware.


> Installername and date/time are unknown.

How hard would that be to remedy?
Many people would say any software that installs something without explicit user approval and notification would qualify in-part as "malware". In any case, if the intent of an installer is to install an add-on, I suspect even theoretically "good" external installs of add-ons would be programed to just override things and enable them. As previously stated, there is no possible way to have control over this.

The best we can hope for here is to have Firefox auto-notify users of external installs when they play nice. Disabling by default would just encourage idiots to disable the disable by default.

(In reply to comment #16)
> > Installername and date/time are unknown.
> 
> How hard would that be to remedy?

Installer name would be essentially impossible to get. Just looking at a file, there's no way to tell who put it there. As to time of install, you could get that on any file systems with file creation times, though I don't see a way to fetch that in JS.
(In reply to comment #17)
> I suspect even
> theoretically "good" external installs of add-ons would be programed to just
> override things and enable them.

That's not true. I support many businesses and most software behaves properly in its environment, from Windows apps, to MacOS apps, to Firefox and Thunderbird add-ons that install via the UI, to IE add-ons. In the case of ffclickonce, the add-on behaved properly; Microsoft didn't hide it or disable the 'Disable' button, and the Uninstall button was disabled by Firefox (for a good reason). Most developers aren't out to trick anyone or hack anyone. They don't have time or inclination, and don't want their business' name and products associated with such things or to incur the cost of the support calls.
I imagine there aren't going to be changes along these lines for Firefox 3.5 -- it's not currently "blocking" the release and therefore unlikely to get an exception to the localization string freeze. Nominating for next time so it doesn't get left out of the planning process.
Flags: blocking1.9.2?
Whiteboard: [sg:want]
Target Milestone: --- → mozilla1.9.2
Need to consider this along with the new UI plans
Keywords: uiwanted
Blocks: 495687
FWIW, this issue is suddenly attracting attention, so there might be an unusually high level of publicity and interest in what Firefox does here.

Slashdot (link not working for me now, but I think it's their website)
http://yro.slashdot.org/story/09/06/01/1438219/Microsoft-Update-Quietly-Installs-Firefox-Extension

Brian Krebs in the Security Fix blog at the Washington Post
http://voices.washingtonpost.com/securityfix/2009/05/microsoft_update_quietly_insta.html
"certainly disabling them by default is not the way we would go."

How about just not executing code until a user says "yes" even enabling a
timer. 

As an application, and increasingly an application platform the evaluation
should not be "what I think the other developer should do is ...". Rather the
evaluation should be taken from the perspective of "How do we assist the user
in avoiding problems arising from X Y and Z" and as a last resort "How do we
ensure accountability for stupid or malicious actions". There might not be a
mechanism for that last one, but you shouldnt avoid trying under the guise of
ease of use.

You dont need to look too far back in the history of browsers to see the
"enable by default" problems that are out there. Easy does not mean right.

/preach
No longer blocks: 495687
Flags: blocking1.9.2? → blocking1.9.2-
I'll be working on some UI mockups.  Overall the various attributes we are weighing are:

1) making sure that the user is informed and has total control over what extensions are added prior to them being enabled (in the event that they are fully engaged and actively care about what code is running on their machine)

2) not blocking disengaged and mainstream users from accessing the Web until they provide an opinion on what they consider to be a remote arcane implementation centric detail that we shouldn't be bothering them with in the first place (obviously this is not our perspective on the issue, but nonetheless a perspective held by a large number of users)

3) not disabling third party add-ons by default for all time, because this effectively makes it impossible for third parties to integrate with Firefox (opt in means only a tiny percentage of users will have the add-on).

The plan is for this work to fit in with all of the other notification redesign work that is going on at the moment so that we provide a consistent interface for notifications throughout the product.
Is there a separate bug for allowing users to disable (for themselves only) system wide addons/plugins?
(In reply to comment #31)
> Is there a separate bug for allowing users to disable (for themselves only)
> system wide addons/plugins?

No, users can already do that.
blocking2.0: --- → ?
(In reply to comment #28)
> 1) making sure that the user is informed and has total control over what
> extensions are added prior to them being enabled
>  [...]
>3) not disabling third party add-ons by default [...]

Is this bug limited to "extensions" proper, or "add-ons" in the larger sense including plug-ins? If this doesn't cover plug-ins then we need to file another bug to make sure they get the same treatment.
(In reply to comment #33)
> (In reply to comment #28)
> > 1) making sure that the user is informed and has total control over what
> > extensions are added prior to them being enabled
> >  [...]
> >3) not disabling third party add-ons by default [...]
> 
> Is this bug limited to "extensions" proper, or "add-ons" in the larger sense
> including plug-ins? If this doesn't cover plug-ins then we need to file another
> bug to make sure they get the same treatment.

It's a little difficult to treat plugins in the same way since we can't detect new plugins at startup without incurring a performance penalty, so we may have to consider how to handle plugins differently. Not sure if it makes sense to have a second bug for plugins straight away though.
(In reply to comment #34)
> It's a little difficult to treat plugins in the same way since we can't detect
> new plugins at startup without incurring a performance penalty

If I'm right in thinking that a plugin isn't actually started until Firefox encounters an embedded object that it handles, perhaps the warning could be shown at that time.
Yes, we shouldn't do plugins at startup anyway, we should do them when somebody first tries to load/instantiate them (tries to instantiate an <object> with a previously-unknown MIME type, since we have to load the plugin DLL to figure out what MIME types it supports).
So just to review the UI we are considering, we are planning to have a start up content area page for extensions, but for plugins use a notification (bar or later doorhanger)?  Plugins will still require a restart if the user chooses to enable, right?
(In reply to comment #37)
> So just to review the UI we are considering, we are planning to have a start up
> content area page for extensions, but for plugins use a notification (bar or
> later doorhanger)?  Plugins will still require a restart if the user chooses to
> enable, right?

I was thinking about this a little earlier. Plugins we have the benefit of being able to load without restarting the application. We also have the downside that on some platforms we have to load them in order to even know what they are and what they support. That probably isn't a massive deal though. One possible UI would be for us to display a message where the plugin would be the first time a page attempts to use it. Something like "This page wants to use Flash which has been installed on your computer. Do you want to start using Flash?". This would be in-page so non-modal and clicking ok could then just load the plugin. I don't really know how technically feasible this is to do right now though.
@Comment #38

I'm fairly certain IE does this in certain situations. I know the first time I visit a site (in IE) with the WMP control I get prompted, for example.
Blocks: 454769
This same thing has happened to me and many other people in the past week with Norton Toolbar/Anti-phishing https://bugzilla.mozilla.org/show_bug.cgi?id=547772 installing itself.

There have been web fixes posted but none of them work now.
You can only disable (for that session) and not delete.
As soon as you reboot, it reappears.

Norton makes it deliberately hard to configure their anti-virus/security
software. They just want you to turn everything "on" and they nag you
if you don't.
blocking2.0: ? → beta1+
Then how about create come ignore list to adding this annoying addons so it wont reappear on Fx restart... ?
Target Milestone: mozilla1.9.2 → ---
Blocks: 565565
When I install a new extension, terminate the application, and then restart the application, the Ad-Ons Manager reports that a new extension has been added.  In the Ad-Ons Manager window, there is a Disable button and an Uninstall button.  This happens for ALL extensions, not merely those not from addons.mozilla.org.  Is this not sufficient to resolve this bug?  Or is it possible to install an ad-on that fails to trigger the report?
Assignee: dtownsend → nobody
I don't think we can block the beta on this right now
blocking2.0: beta1+ → -
I believe we should change this bug to "Warn the user and allow enabling."  Comprehension of add-ons shouldn't be a prerequisite for preventing a third party to modify Firefox without the user's intent.
I'd like to see a warning message like about:config when an add-on is installed:

> This might void your warranty!
> Changing these advanced settings can be harmful to the stability, security, 
> and performance of this application. You should only continue if you are sure 
> of what you are doing.
Is it really that difficult to come up with a dialog like "hi there, someone using the same computer as you with another username or even the administrator installed addon XYZ, do you want to use that addon too?" Yes / No / I'm feeling lucky.
That this is known for over a year now and hasn't been changed in a way annoys me so bad because my beloved root installed a addon that makes firefox behave like it was VIM (vimperator) and I had to figure out how to disable that damn **** (of course root was not at home because addon showed after firefox iceweasel "rebooted" so it was the next day) 
yeah type ":addons" to get to that dialog isn't so intuitive and if I would never had used VI or VIM before I would have had to read that whole start page of that addon (printed it would be probably more than 3 pages) to get a clue what to do. There are no more menus at all with this special addon. That makes it very difficult for a regular user. So please, hooray for a tiny dialog.
Apparently through bug 596343 it already does block Firefox 4
blocking2.0: - → betaN+
This bug smells like strings just as much as bug 596343 does, or are they
the same strings?
No they are very likely different, though we don't really have UI designs yet for either to be sure
Whiteboard: [sg:want] → [sg:want][strings]
A big question here is whether add-on installation by third-parties should default to installing or not installing if the user ignores the dialog.  I agree with Comment 17 that if something entirely other than the what the user has previously given permission for is installed without asking permission, it could be considered malware.  In Firefox, we only silently update continuations of items the user has already consented to, such as add-ons and Firefox itself.   

Arrow notifications in Firefox 4.0 are very easy to ignore.  If the user clicks anywhere on the page outside of the dialog box, it's effectively a "cancel" to the notification.  Because some users don't know this is possible, some currently feel forced to click the only option on the box, which in this case would be "Add to Firefox."  This is an issue we're separately addressing, but for notifications with consequences as large as add-on installation, using a content area message with a Cancel option rather than an arrow notification would avoid users feeling forced to agree to installation.

I'm attaching a mockup of a default-to-no notification.  If the user does not click "Install," the add-on never runs.  This should work similarly to current add-on installation, when the file is downloaded but not read until the user gives permission.

(Note: I don't have the final graphics for content area messages, so the attached mockup is just a modified arrow notification to show how it the message could be phrased.)
Will this include plugins?
In bug 596343, you dropped the "only authors you trust" for application-installed add-ons, should that be done here as well?
Also how would it look like when multiple 3rd party add-ons will be installed? Are those listed in the same notification, or do we popup multiple notifications sequentially?
(In reply to comment #52)
> Apparently through bug 596343 it already does block Firefox 4

This bug scares me a lot in terms of potential for downstream surprises and edge cases. If it blocks at all at this late stage of the game, we should at least make the decision explicitly instead of in terms of "blocks a blocker."

Dave - you're the toolkit module owner and the expert on the EM - what's your opinion, here? Is this safe and containable work?
(In reply to comment #60)
> (In reply to comment #52)
> > Apparently through bug 596343 it already does block Firefox 4
> 
> This bug scares me a lot in terms of potential for downstream surprises and
> edge cases. If it blocks at all at this late stage of the game, we should at
> least make the decision explicitly instead of in terms of "blocks a blocker."
> 
> Dave - you're the toolkit module owner and the expert on the EM - what's your
> opinion, here? Is this safe and containable work?

I think we're generally ok in terms of this being containable. We will have to tightly restrict what add-ons we do this for (we cannot do it for plugins right now for example) and accept that it probably won't work for certain cases (Skype I believe uses direct injection into the profile directory). It will be difficult to do automated testing for this and it's not the sort of thing that I'd anticipate being experienced often by nightly testers so coverage will not be as good as we'd probably like.

My biggest problem with doing this this late is that we can't do it properly. Even the simple mockups that Boriss has proposed aren't feasible since we have no idea what installed the new add-on and the question isn't whether to install it, it is whether to enable it.

The right thing to do here is probably to switch to a different mechanism for third parties to install add-ons into Firefox, one that tells us where it came from and gives us control over uninstalling it when no longer needed, but that is a longer term project, not one we can do this late in the cycle since it'd require application developers to make changes to how they deploy their software to support it.

But I guess this is a semi-useful stepping stone and it is certainly true that bug 596343 is really a waste of time without this. I think if we insist on blocking on that then this probably does need to be a blocker, which is still making it a blocker by blocking a blocker but there you go.
(In reply to comment #55)
> Created attachment 492249 [details]
> Mockup: In-content notification of a third-party add-on installation
> 
> Arrow notifications in Firefox 4.0 are very easy to ignore.  If the user clicks
> anywhere on the page outside of the dialog box, it's effectively a "cancel" to
> the notification.  Because some users don't know this is possible, some
> currently feel forced to click the only option on the box, which in this case
> would be "Add to Firefox."  This is an issue we're separately addressing, but
> for notifications with consequences as large as add-on installation, using a
> content area message with a Cancel option rather than an arrow notification
> would avoid users feeling forced to agree to installation.

This is kind of confusing and doesn't marry in my head with the plans to move the normal add-on install dialog to a doorhanger in the future (bug 588266). Surely if we are concerned with users feeling the need to install add-ons presented in doorhangers then we shouldn't do that?

> I'm attaching a mockup of a default-to-no notification.  If the user does not
> click "Install," the add-on never runs.  This should work similarly to current
> add-on installation, when the file is downloaded but not read until the user
> gives permission.

As alluded to in comment 61 the question we need to ask is not whether to install the add-on but whether to enable it since right now we don't support uninstalling these kinds of add-ons. We also don't know who or what installed the add-on, we just know it is there now and wasn't the last time Firefox started. I think we also need to have a way for the user to restart Firefox at this point since that would normally be needed to enable the add-on in question.

We have to consider what we do on a user's first run of Firefox when there may already be add-ons present on their system also as Henrik mentioned the UI needs to support listing multiple add-ons.

(In reply to comment #57)
> Will this include plugins?

Not for Firefox 4 unless we want to take a Ts hit (I'm not sure how we could ever do it without a Ts hit but it'd certainly require some clever voodoo in the plugin host which is too risky right now)
From the QA perspective I could crawl through the list of add-ons I have assembled for the dll blocklisting project for Firefox 3.6. I think it should give us a good number of add-ons, which get installed globally. Here the list:

https://wiki.mozilla.org/QA/Firefox3.6/TestPlan:DLL_Blocklisting:3rd-party
(In reply to comment #58)
> In bug 596343, you dropped the "only authors you trust" for
> application-installed add-ons, should that be done here as well?

I'd keep the phrase here here, mostly for symmetry with standard add-on installation.  Removing the phrase from third-party installation but including it in manual installation implies that there's less reason to trust manual installs than third-party.  Also, removing the phrase in bug 596343 partially serves to make a potentially divisive action less offensive to application authors.

(In reply to comment #59)
> Also how would it look like when multiple 3rd party add-ons will be installed?
> Are those listed in the same notification, or do we popup multiple
> notifications sequentially?

Hopefully this case is super-super-rare, but sequentially should cover it.

(In reply to comment #61)
> (In reply to comment #60)
> > (In reply to comment #52)
> > > Apparently through bug 596343 it already does block Firefox 4
> I think we're generally ok in terms of this being containable. We will have to
> tightly restrict what add-ons we do this for (we cannot do it for plugins right
> now for example) and accept that it probably won't work for certain cases
> (Skype I believe uses direct injection into the profile directory). It will be
> difficult to do automated testing for this and it's not the sort of thing that
> I'd anticipate being experienced often by nightly testers so coverage will not
> be as good as we'd probably like.
> 
> My biggest problem with doing this this late is that we can't do it properly.
> Even the simple mockups that Boriss has proposed aren't feasible since we have
> no idea what installed the new add-on and the question isn't whether to install
> it, it is whether to enable it.

I was afraid of that.  We can always say "an application has requested," but not revealing the source is potentially problematic.  If "an application" requests the install of a malicious add-on called "Firefox security update" with a Firefox icon (if we can even display the add-on on this screen?), it's not awesome.  Still an improvement on the current case, though.

> The right thing to do here is probably to switch to a different mechanism for
> third parties to install add-ons into Firefox, one that tells us where it came
> from and gives us control over uninstalling it when no longer needed, but that
> is a longer term project, not one we can do this late in the cycle since it'd
> require application developers to make changes to how they deploy their
> software to support it.

Agreed

(In reply to comment #62)
> (In reply to comment #55)
> > Created attachment 492249 [details] [details]
> > Mockup: In-content notification of a third-party add-on installation
> > 
> > Arrow notifications in Firefox 4.0 are very easy to ignore.  If the user clicks
> > anywhere on the page outside of the dialog box, it's effectively a "cancel" to
> > the notification.  Because some users don't know this is possible, some
> > currently feel forced to click the only option on the box, which in this case
> > would be "Add to Firefox."  This is an issue we're separately addressing, but
> > for notifications with consequences as large as add-on installation, using a
> > content area message with a Cancel option rather than an arrow notification
> > would avoid users feeling forced to agree to installation.
> 
> This is kind of confusing and doesn't marry in my head with the plans to move
> the normal add-on install dialog to a doorhanger in the future (bug 588266).
> Surely if we are concerned with users feeling the need to install add-ons
> presented in doorhangers then we shouldn't do that?

It's possible we shouldn't.  TBH I only found out that in-page notifications have cancel buttons when they landed yesterday.  Something to discuss offline.

> > I'm attaching a mockup of a default-to-no notification.  If the user does not
> > click "Install," the add-on never runs.  This should work similarly to current
> > add-on installation, when the file is downloaded but not read until the user
> > gives permission.
> 
> As alluded to in comment 61 the question we need to ask is not whether to
> install the add-on but whether to enable it since right now we don't support
> uninstalling these kinds of add-ons. We also don't know who or what installed
> the add-on, we just know it is there now and wasn't the last time Firefox
> started. I think we also need to have a way for the user to restart Firefox at
> this point since that would normally be needed to enable the add-on in
> question.

Yes, at this stage the add-on is installed but disabled.  Enabling it would require a restart.  I can add a flow diagram to make that more clear.
(In reply to comment #64)
> I was afraid of that.  We can always say "an application has requested," but
> not revealing the source is potentially problematic.  If "an application"
> requests the install of a malicious add-on called "Firefox security update"
> with a Firefox icon (if we can even display the add-on on this screen?), it's
> not awesome.  Still an improvement on the current case, though.

I wouldn't spend too much time worrying about security concerns here, since this is software that already has access to the user's computer. If they want to replace the Firefox binary with their own, they can already do that. This should simply be about preventing annoyances that third-party software has installed, like the various toolbars. Real malware will continue to find ways to do what it's doing regardless of what we do.
>This is kind of confusing and doesn't marry in my head with the plans to move
>the normal add-on install dialog to a doorhanger in the future

The doorhangers are designed to be associated with site identity (the door knob, to carry the analogy forward).  In this case, the "identity" of the party asking permission is local software, and not a web site, so we don't have an identity block to associate it with (and using a doorhanger off of the user's home page would be confusing, especially if they have customized their home page).  Also, we need to capture the user's attention enough that they have a chance to opt in, so a content area UI message (similar to about:sessionrestore) seems to be the best balance for this notification given all of the current available mechanisms.
Asa Dotzler of Spread Firefox blogs about this problem at <http://weblogs.mozillazine.org/asa/archives/2010/11/why_do_they_think_th.html>.  Dotzler's attack against Microsoft, Google, Apple, and others was then picked up by online magazine PC Pro at <http://www.pcpro.co.uk/news/363196/apple-microsoft-and-google-attacked-for-evil-plugins>, which was then cited by SlashDot.
If you read his post carefully, you will find out that he is not talking about this problem at all, but about actual NPAPI plugins being installed by third-party software. This bug is about extensions.
Comment on attachment 492251 [details]
Mockup: In-content notification of a third-party add-on installation

I like the way that this makes it feel like Firefox is asking for permission to install the add-on, even though behind the scenes what we'd be doing is disabling the already-installed add-on. Nice UX model interpretation of the implementation model.
Attachment #492251 - Flags: ui-review+
Curious why it's believed to be better to make it feel like Firefox is asking for permission to *install* rather than *enable* the add-on? 

It seems like asking "Would you like to enable it?" would be more accurate and honest about what's going on without changing much, if anything of the perceived implications of the action by the user.
(Just so I'm not sending the wrong message here, I *really* like the dialog and think this is a huge step forward regardless of wording. It just seems that the spirit of the dialog would be the same even if we went for a 100% accurate wording.)
>Curious why it's believed to be better to make it feel like Firefox is asking
>for permission to *install* rather than *enable* the add-on? 

From the user's perspective this may be the first time they are hearing about the new extension, so the fact that it is already installed is kind of rude (albeit how the system currently works).  Imagine you sat down at your computer and found out that new local applications had been installed (and would you now like to enable them?).  Also, enable something already installed sounds a little more like we are endorsing the extension.
(In reply to comment #72)
> From the user's perspective this may be the first time they are hearing about
> the new extension, so the fact that it is already installed is kind of rude
> (albeit how the system currently works).

But the fact that it is how it works is, I think, the issue. If the user says they don't want to install it, and then goes into the add-ons list, they will find it is installed, but disabled. If they never get that far, I guess they might feel better about the process. If they do, they will find out that Firefox lied to them, which they may then be annoyed or confused by.
(In reply to comment #73)
> (In reply to comment #72)
> > From the user's perspective this may be the first time they are hearing about
> > the new extension, so the fact that it is already installed is kind of rude
> > (albeit how the system currently works).
> 
> But the fact that it is how it works is, I think, the issue. If the user says
> they don't want to install it, and then goes into the add-ons list, they will
> find it is installed, but disabled. If they never get that far, I guess they
> might feel better about the process. If they do, they will find out that
> Firefox lied to them, which they may then be annoyed or confused by.

I tend to agree with this. I'd prefer the UI to be more focused on whether you want to allow the new add-on to run or not.
Changed the title of the bug to reflect what the consensus for behavior seems to be: load it as disabled by default, allow user to enable it)

Faaborg is the owner of this currently, AFAIK, but I'll try to summarize:

* We want third-party extensions to be opt-in, and disabled by default (even if it means restarting the browser to enable them).

* We don't have the ability to know which application added the extension, this is ok.

* An in-content page is better for a number of reasons:
  * Doorhanger is optimized for a single, obvious answer and an easy way to dismiss; this is neither of those,
  * An in-content page makes it easier to handle multiple extension installs at once — if more than one app has added an extension, showing several doorhangers sequentially or stacking them simultaneously doesn't work very well
  * Being able to postpone the decision easily ("it's just a tab")
  * Doorhanger is about a website telling you things, NOT a message from Firefox.

Faaborg just confirmed that he'll make a wireframe for this. Boriss is busy this week because of Add-on Con etc, so I'm hereby handing it to Faaborg. :)
Summary: Warn user that a third party add-on has been installed and allow disabling → Make third-party add-ons disabled on startup, and allow users to activate them
If the user says that they don't want the add-on installed, then it should not only be disabled, but it should be hidden from view in the Add-Ons list. While I understand and sympathize that this isn't how it actually happens, the user doesn't need to know that. It is an abstraction, not a lie.

Let's not expose the implementation model limitations here, and instead build the user experience to match the user's mental model.

Faaborg/Limi: perhaps instead of a new in-content page, we could simply open the Add-Ons manager in a tab and have the doorhanger appear off that. This is what I expected when I first saw the mockup, and saves a lot of work.
(In reply to comment #76)
> If the user says that they don't want the add-on installed, then it should not
> only be disabled, but it should be hidden from view in the Add-Ons list. While
> I understand and sympathize that this isn't how it actually happens, the user
> doesn't need to know that. It is an abstraction, not a lie.
> 
> Let's not expose the implementation model limitations here, and instead build
> the user experience to match the user's mental model.

So just to be clear you are saying that as well as implementing this UI we also need to make add-ons that the user didn't opt in to hidden in the add-ons manager?
Unrelated to the wording/behavior: Would it make sense to add a "Why is Firefox asking me this question?" / "Learn more" type of link that points to a related SUMO article explaining what just happened?

The mockup dialog does a good job of pointing out that an external app wanted to install this add-on, but it might still be confusing to some that Firefox is asking this question.

In any case, we'd be happy to write an support article for it if you think it'd be useful. It's hard to tell how users might react, which is why I'm throwing this out there.

(In reply to comment #76)
> If the user says that they don't want the add-on installed, then it should not
> only be disabled, but it should be hidden from view in the Add-Ons list. While
> I understand and sympathize that this isn't how it actually happens, the user
> doesn't need to know that. It is an abstraction, not a lie.
> 
> Let's not expose the implementation model limitations here, and instead build
> the user experience to match the user's mental model.
> 
> Faaborg/Limi: perhaps instead of a new in-content page, we could simply open
> the Add-Ons manager in a tab and have the doorhanger appear off that. This is
> what I expected when I first saw the mockup, and saves a lot of work.

so what if the user changes his mind... and wants the add-on after a while.
the add-on is installed, but disabled and hidden.
Can he reinstall and see it again.
What if it comes with like the .net add-on by MS, would he have to install the program again.

I don't like the idea of hiding it unless there is an easy way to unhide
>so what if the user changes his mind

We've been discussing sorting the list of extensions in the add-ons manager first by enabled/disabled, and then alphabetically.  This way all of the cruft would settle at the bottom (both from the opt-in stage during major update, and for any subsequent opt-ins as local applications attempt to install things).  Also, users would then have total control over undoing any decisions they have previously made.

>What if it comes with like the .net add-on by MS, would he have to install the
>program again.

yeah, and sometimes the order in which one installs programs in Windows is actually important due to dependency chains.  Worst case scenario you have to reinstall windows and then the series of programs in order.
Updated mockup showing the use of a content area message at launch (similar to about:sessionrestore):

http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/addOnLocalInstall-i1.png
>Would it make sense to add a "Why is Firefox
>asking me this question?" / "Learn more" type of link

Yeah, I think this would be fine.
>Faaborg/Limi: perhaps instead of a new in-content page, we could simply open
>the Add-Ons manager in a tab and have the doorhanger appear off that.

We want the user to focus on the question being asked, so this task requires a more modal design.  Doorhangers were meant to respond to a user action (clicking a link, loading a page).  If one appears at start up, it likely won't be the first thing the user notices since so many things are changing simultaneously.  Also, they are too easy to dismiss for a critical question like this, and in the case of the add-ons manager, there isn't any anchor to logically associate them with (this is app-based instead of site-based).  While it doesn't initially sound like a positive analogy, these content area messages are somewhat based on the pattern used by UAC, single focused yes/no modal questions.
To comment 77, yes, ideally we'd hide them in some fashion. As you and others point out, though, there's a hard limitation here in that once hidden, there's no way to tell if a user has re-installed the third-party application in order to try and re-install that add-on. Is there a way that would happen now, though? Or is it the case that users would need to dig into the Add-Ons Manager to re-enable the Add-on?

The wording, however, should reference installation. That we merely disable it is still very much an implementation detail. I do not think anyone will be upset or felt lied to, but to mitigate that possibility (and since we can't hide the Add-On" we could change the action verbs to be "Allow Installation" and "Disable"

Alex: In either case, feels like we need to handle the "user didn't answer" case, though. Unless that would be the only tab that loaded until an answer was given.
New version: http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/addOnLocalInstall-i2.png

This adds the help link, and makes it easier for users who are rapidly looking for a whatever button to opt-out, instead of accidentally opting in without actual consent.
(In reply to comment #84)
> To comment 77, yes, ideally we'd hide them in some fashion. As you and others
> point out, though, there's a hard limitation here in that once hidden, there's
> no way to tell if a user has re-installed the third-party application in order
> to try and re-install that add-on. Is there a way that would happen now,
> though? Or is it the case that users would need to dig into the Add-Ons Manager
> to re-enable the Add-on?

The best we could do is check the file modification times of the add-ons files. If they change then we could assume that the application had been reinstalled and so present this UI again. This isn't perfect though, it's possible an application could install its add-on with fixed modification times (wouldn't surprise me at all actually) in which case we'd miss it. Likewise if an application got updated not reinstalled it may modify the add-ons modification times in which case we'd re-present the UI to the user.

> The wording, however, should reference installation. That we merely disable it
> is still very much an implementation detail. I do not think anyone will be
> upset or felt lied to, but to mitigate that possibility (and since we can't
> hide the Add-On" we could change the action verbs to be "Allow Installation"
> and "Disable"

This makes sense to me.

> Alex: In either case, feels like we need to handle the "user didn't answer"
> case, though. Unless that would be the only tab that loaded until an answer was
> given.

When we first discussed this (sometime ago) we had suggested that no answer constituted enabling the add-on on the next restart, my belief at the time was that this would feel nicer to the applications and make them less likely to take more extreme steps to get themselves installed and enabled. Not totally sure I still feel the same, just saying what was the case before.
>When we first discussed this (sometime ago) we had suggested that no answer
>constituted enabling the add-on on the next restart

I really think that no answer could be more accurately parsed as the user is not actually interested in installing the add-on.
(In reply to comment #87)
> I really think that no answer could be more accurately parsed as the user is
> not actually interested in installing the add-on.

I agree. If you don't know what it is, and you don't miss it (ie. don't go looking for it), then it's a pretty good bet that you don't care about it.
(In reply to comment #85)
> New version:
> http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/addOnLocalInstall-i2.png

I think this is in general an elegant solution and looks great.

One minor nit: the tab title should be more explanatory. "Install Add-on" sounds like it's just a matter of fact. I'd much prefer "External Add-on Install" to better drive home the fact that this is originating outside of Firefox.

(In reply to comment #80)
> We've been discussing sorting the list of extensions in the add-ons manager
> first by enabled/disabled, and then alphabetically.

This is a very good idea that would increase usability dramatically. To extend it, this could be separated into three groups instead of just two by adding a third at the very bottom for disabled/undecided external installs arising from this system. This would show the addons the user cares about up top, scrolling down to the ones they had involvement with below that, and leaving the ignored cruft at the bottom not even mixed in with the other disabled addons which the user at least cared enough about to disable but not uninstall, implying they may care to enable them again. (should any external addon approved by the user then later be disabled, I would think it should go into bucket 2 instead of the bottom, but that's debatable)
Blocks: 618254
(In reply to comment #82)
> >Would it make sense to add a "Why is Firefox
> >asking me this question?" / "Learn more" type of link
> 
> Yeah, I think this would be fine.

Filed bug 618254 for the "Learn More" page.

(In reply to comment #85)
> New version:
> http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/addOnLocalInstall-i2.png

Attachment 492251 [details] also mentioned the name of the installer triggering this add-on install, but it looks like this piece of info was left out of the last version. If it's possible to keep that info in the dialog, it would be preferable as that helps the user with the evaluation.
The installer name unfortunately cannot be ascertained, which is why it was removed from the early mockups:

(In reply to comment #17)
> Installer name would be essentially impossible to get. Just looking at a file,
> there's no way to tell who put it there. As to time of install, you could get
> that on any file systems with file creation times, though I don't see a way to
> fetch that in JS.
Is there anything we could do in the future to try to figure out the installer?
(In reply to comment #92)
> Is there anything we could do in the future to try to figure out the installer?

Not really with the current mechanism for adding add-ons. I think the better way forward is to move to a situation where installers place the add-on and some kind of additional manifest somewhere on the system and on startup Firefox reads the manifest to tell it what application is providing the add-on and lets us offer to install the add-on in a way that we can uninstall later at the user's request.
Note: updated the text in i2 to read "Install add-ons only from authors whom you trust" to reduce any potential grammatical ambiguity.
If an add-on that I don't want is installed, I don't want that fact hidden from me.  Eve if such an add-on is disabled, it should be brought to my attention so that I can remove it.  

Better yet, the old design is what I really want.  No add-on should be installed unless I give permission.  This was controlled by the preference variable xpinstall.enabled; if set "false", installation of any add-on was blocked.  If I would then permit an add-on to be installed, I of course would want it enabled.  I still cannot understand why that design is being abandoned.
I am fairly certain the xpinstall.enabled pref never prevented installing add-ons via the registry or via placing the add-on into one of the add-on directories
Notes from the Grand Retriage: this needs a call from product, provisionally minusing.
blocking2.0: betaN+ → -
Whiteboard: [sg:want][strings] → [sg:want][strings][d?]
The call from product is that the various mitigations being discussed here make me feel like we should punt.

The solution needs to be one such that:
 - the user is asked BEFORE the add-on is installed and activated
 - 3rd parties can update add-ons if they're installed
 - if the user says no, it shouldn't appear in the add-ons manager list
Whiteboard: [sg:want][strings][d?] → [sg:want][strings]
Depends on: 643020
Blocks: 643020
No longer depends on: 643020
re comment 88:
> I agree. If you don't know what it is, and you don't miss it (ie. don't go
> looking for it), then it's a pretty good bet that you don't care about it.

could you possibly use this concept in the message?

just because a user trusts Adobe, Microsoft or Skype doesn't mean that they *need* an add-on from them.

~ If you aren't familiar with this add-on you probably can live without it. You should only install an add-on from someone you trust if you know you need it. ~

A link to some page which explains how extensions can harm Firefox's performance could be useful.

Also, could someone please provide a mockup which shows 2 or more addons?

I just returned to a computer for the first time in over a month, it has 6 add-ons. Plus it had update notifications for another 6 programs. So while we like to think users will only see one new thing at a time, this isn't necessarily the case.

The simplest approach to this is to have each tab include the name of the add-on.

Install "CaffeinatedFox" Add-On

----

I'd like to remind people that in some circumstances the user who installed the add-on is an administrator (or network administrator) and not the user running Firefox who will see the content we're presenting. -- the ui designers working on this bug seem to be aware of this, which is reassuring :)
Whiteboard: [sg:want][strings] → [sg:want]
Assignee: nobody → dtownsend
Would this apply to EVERY startup?  Or would it apply only to the first startup after the add-on is installed?
Just when it was first installed
Depends on: 656125
Depends on: 666431
Depends on: 666437
Going to have to clone the in-content button styles here unless bug 658530 is fixed first
Depends on: 658530
(In reply to comment #103)
> Going to have to clone the in-content button styles here unless bug 658530
> is fixed first

Or just leave them with native styling. Once the in-content patch in bug 658530 lands, it'll all just magically look pretty. With ponies and rainbows and stuff.
Keywords: ux-control
Depends on: 670033
Is it possible to give installed extension the ability to present itself with more than just a small icon. Let's say, install.rdf will have url to some small HTML page, that will be presented instead of the icon (optionally if exists)
In another words, to customize the inner box.
(In reply to comment #105)
> Is it possible to give installed extension the ability to present itself
> with more than just a small icon. Let's say, install.rdf will have url to
> some small HTML page, that will be presented instead of the icon (optionally
> if exists)
> In another words, to customize the inner box.

Not in this initial pass, it's something I've been considering for the future.
Depends on: 677424
Attachment #492251 - Attachment is obsolete: true
Attached patch patch rev 1 (obsolete) — Splinter Review
This patch implements the UI for this and makes any add-on dropped into the third-party locations default to disabled. There is one bug, right now the UI gets restored after a restart due to bug 677424. This isn't too bad as the UI detects the add-on is already enabled and closes itself. On my system that happens so fast you don't even see it but hopefully we'll get a fix for bug 677424 too.
Attachment #551631 - Flags: review?(robert.bugzilla)
Comment on attachment 551631 [details] [diff] [review]
patch rev 1

Review of attachment 551631 [details] [diff] [review]:
-----------------------------------------------------------------

BOOM! DRIVEBY REVIEW!

Feels like the navbar should be hidden for about:newaddon.

::: browser/app/profile/firefox.js
@@ +73,5 @@
>  pref("extensions.blocklist.itemURL", "https://addons.mozilla.org/%LOCALE%/%APP%/blocked/%blockID%");
>  
>  pref("extensions.update.autoUpdateDefault", true);
>  
> +// Disable add-ons installed into the shared user and system areas by default

Would be nice to have the values explained here.

::: toolkit/locales/en-US/chrome/mozapps/extensions/newaddon.dtd
@@ +1,4 @@
> +<!ENTITY title           "Install Add-on">
> +<!ENTITY intro           "Another program on your computer would like to modify
> +                          &brandShortName; with the following add-on:">
> +<!ENTITY warning         "Install add-ons only from authors whom you trust">

Needs a full-stop.

@@ +3,5 @@
> +                          &brandShortName; with the following add-on:">
> +<!ENTITY warning         "Install add-ons only from authors whom you trust">
> +<!ENTITY allow           "Allow this installation">
> +<!ENTITY later           "You can always change your mind at any time by going
> +                          to the Add-ons Manager">

Needs a full-stop. 

Also, I wonder if "Add-ons Manager" should be a link to open the Add-ons Manager - it would act as a way to introduce people to the Add-ons Manager.

@@ +5,5 @@
> +<!ENTITY allow           "Allow this installation">
> +<!ENTITY later           "You can always change your mind at any time by going
> +                          to the Add-ons Manager">
> +<!ENTITY continue        "Continue">
> +<!ENTITY restartMessage  "You must restart &brandShortName; to finish installing this add-on">

Needs a full-stop.

::: toolkit/mozapps/extensions/content/newaddon.js
@@ +71,5 @@
> +    // enabled then this UI is useless, just close it. This shouldn't normally
> +    // happen unless session restore restores the tab
> +    if (!aAddon || !aAddon.userDisabled ||
> +        !(aAddon.permissions & AddonManager.PERM_CAN_ENABLE)) {
> +      window.close();

For some reason, it won't close itself if passed an empty ID.

@@ +86,5 @@
> +    let name = bundle.formatStringFromName("name", [aAddon.name, aAddon.version],
> +                                           2);
> +    document.getElementById("name").value = name
> +
> +    if (aAddon.author) {

Think you mean aAddon.creator here. Did the test not pick that up?

::: toolkit/mozapps/extensions/content/newaddon.xul
@@ +66,5 @@
> +      <image id="icon"/>
> +      <vbox flex="1">
> +        <label id="name"/>
> +        <label id="author"/>
> +        <label id="location" crop="end"/>

If this can be cropped, it should also have the tooltiptext set.

::: toolkit/themes/pinstripe/mozapps/extensions/newaddon.css
@@ +130,5 @@
> +#later {
> +  color: GrayText;
> +}
> +
> +.addon-control {

The additional inContentUI stuff landed (bug 658530), so this isn't needed anymore (same on winstripe). The buttons also don't need a class to pickup the in-content style.
Attachment #551631 - Flags: feedback+
(In reply to Blair McBride (:Unfocused) from comment #108)
> Feels like the navbar should be hidden for about:newaddon.

I was thinking this too; this dialog seems extremely spoofable, though I can't think of any really good tricks to play with a fake 3rd party install page.

I wish we could also display some more information about the add-on to help people decide if they want it, like its description and author name (especially since we say "Install add-ons only from authors whom you trust") but it's probably not necessary for the first implementation.
(In reply to Blair McBride (:Unfocused) from comment #108)
> @@ +86,5 @@
> > +    let name = bundle.formatStringFromName("name", [aAddon.name, aAddon.version],
> > +                                           2);
> > +    document.getElementById("name").value = name
> > +
> > +    if (aAddon.author) {
> 
> Think you mean aAddon.creator here. Did the test not pick that up?

Good catch. The test was using a mock add-on that also used the incorrect property name so it wasn't catching it.
Attached patch patch rev 2 (obsolete) — Splinter Review
Updated. Do you just want to do the final review here Blair since you've looked at it already
Attachment #551631 - Attachment is obsolete: true
Attachment #552486 - Flags: review?(bmcbride)
Attachment #551631 - Flags: review?(robert.bugzilla)
> Feels like the navbar should be hidden for about:newaddon.

yeah this is the type of all chrome UI where we would like to hide the navbar.  It makes the interface cleaner so the user can focus on the question, and reduces the number of alternative targets (like the search field) for what is otherwise essentially a dialog box.
(In reply to Alex Faaborg [:faaborg] (Firefox UX) from comment #112)
> > Feels like the navbar should be hidden for about:newaddon.
> 
> yeah this is the type of all chrome UI where we would like to hide the
> navbar.  It makes the interface cleaner so the user can focus on the
> question, and reduces the number of alternative targets (like the search
> field) for what is otherwise essentially a dialog box.

I agree of course, however I don't think it is worth blocking the initial landing on it. If you think we wouldn't ship a Firefox release where the UI was still visible for this page then let me know.
follow up bug for a later release is fine, just commented here since I saw the screenshots on the post that went out.
(In reply to Blair McBride (:Unfocused) from comment #108)
> ::: toolkit/mozapps/extensions/content/newaddon.js
> @@ +71,5 @@
> > +    // enabled then this UI is useless, just close it. This shouldn't normally
> > +    // happen unless session restore restores the tab
> > +    if (!aAddon || !aAddon.userDisabled ||
> > +        !(aAddon.permissions & AddonManager.PERM_CAN_ENABLE)) {
> > +      window.close();
> 
> For some reason, it won't close itself if passed an empty ID.

If the id is empty then getAddonByID will pass back a null aAddon so it should close, right?
(In reply to Justin Scott [:fligtar] from comment #109)
> I wish we could also display some more information about the add-on to help
> people decide if they want it, like its description and author name
> (especially since we say "Install add-ons only from authors whom you trust")
> but it's probably not necessary for the first implementation.

It doesn't currently expose the author link, that could help somewhat.
Attached patch patch rev 3Splinter Review
From comments in IRC:

Added a role attribute
Added scrollbars when the window gets too small (needed to hide the xhtml link element by default since it messes up XUL layout)
Fixed closing when passed an empty ID
Attachment #552486 - Attachment is obsolete: true
Attachment #552570 - Flags: review?(bmcbride)
Attachment #552486 - Flags: review?(bmcbride)
(In reply to Blair McBride (:Unfocused) from comment #116)
> (In reply to Justin Scott [:fligtar] from comment #109)
> > I wish we could also display some more information about the add-on to help
> > people decide if they want it, like its description and author name
> > (especially since we say "Install add-ons only from authors whom you trust")
> > but it's probably not necessary for the first implementation.
> 
> It doesn't currently expose the author link, that could help somewhat.

We'd have to ping AMO to get that, not out of the question but delays when we could display the page which would complicate matters
Attachment #552570 - Flags: review?(bmcbride) → review+
Comment on attachment 552570 [details] [diff] [review]
patch rev 3

Rob, can I get a quick review on the browser parts here
Attachment #552570 - Flags: review?(robert.bugzilla)
Comment on attachment 552570 [details] [diff] [review]
patch rev 3

>--- a/toolkit/themes/gnomestripe/global/inContentUI.css
>+++ b/toolkit/themes/gnomestripe/global/inContentUI.css

>+/* HTML Link elements mess up XUL layout */
>+html|link {
>+  display: none;
>+}

This doesn't belong in themes. If there's no appropriate content stylesheet, you could just add style="display:none" here:

>+  <xhtml:link rel="shortcut icon"
>+              href="chrome://mozapps/skin/extensions/extensionGeneric-16.png"/>
(In reply to Dão Gottwald [:dao] from comment #120)
> Comment on attachment 552570 [details] [diff] [review]
> patch rev 3
> 
> >--- a/toolkit/themes/gnomestripe/global/inContentUI.css
> >+++ b/toolkit/themes/gnomestripe/global/inContentUI.css
> 
> >+/* HTML Link elements mess up XUL layout */
> >+html|link {
> >+  display: none;
> >+}
> 
> This doesn't belong in themes. If there's no appropriate content stylesheet,
> you could just add style="display:none" here:
> 
> >+  <xhtml:link rel="shortcut icon"
> >+              href="chrome://mozapps/skin/extensions/extensionGeneric-16.png"/>

Blair explicitly asked for it in the inContentUI stylesheet as it's likely to cause confusion with other pieces of UI we build in this way which I pretty much agree with. Is there a strong reason to require every page to use display:none or add a new content stylesheet just for this?
(In reply to Dave Townsend (:Mossop) from comment #121)
> Blair explicitly asked for it in the inContentUI stylesheet as it's likely
> to cause confusion with other pieces of UI we build in this way which I
> pretty much agree with. Is there a strong reason to require every page to
> use display:none or add a new content stylesheet just for this?

Yes, we don't want the layout to break depending on whether someone pulls or doesn't pull chrome://global/skin/inContentUI.css.
A third option besides inline CSS and a dedicated content stylesheet might be to add * > html|link { display: none } to xul.css.
Depends on: 678532
(In reply to Dão Gottwald [:dao] from comment #122)
> (In reply to Dave Townsend (:Mossop) from comment #121)
> > Blair explicitly asked for it in the inContentUI stylesheet as it's likely
> > to cause confusion with other pieces of UI we build in this way which I
> > pretty much agree with. Is there a strong reason to require every page to
> > use display:none or add a new content stylesheet just for this?
> 
> Yes, we don't want the layout to break depending on whether someone pulls or
> doesn't pull chrome://global/skin/inContentUI.css.
> A third option besides inline CSS and a dedicated content stylesheet might
> be to add * > html|link { display: none } to xul.css.

Sounds fair. I've filed bug 678532 to add it to xul.css. I'll probably land this with just display:none on the element for now though as I'm a little wary about making a change to xul.css so close to the aurora merge.
Per Mossop, this will do the following:
"Multiple tabs open, one for each. If you click restart in one then the other tabs get restored automatically"

I assume they will be in the same state as they were previously vs. just disabling.

This is sub-optimal since the user will have the option to make the decision regarding one add-on only to be presented with making the decision regarding a different add-on. I don't think it should block getting this landed but this behavior should be evaluated for improvement.
Comment on attachment 552570 [details] [diff] [review]
patch rev 3

>diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js
>--- a/browser/app/profile/firefox.js
>+++ b/browser/app/profile/firefox.js
>@@ -69,16 +69,20 @@ pref("extensions.blocklist.interval", 86
> // blocking them.
> pref("extensions.blocklist.level", 2);
> pref("extensions.blocklist.url", "https://addons.mozilla.org/blocklist/3/%APP_ID%/%APP_VERSION%/%PRODUCT%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/%PING_COUNT%/%TOTAL_PING_COUNT%/%DAYS_SINCE_LAST_PING%/");
> pref("extensions.blocklist.detailsURL", "https://www.mozilla.com/%LOCALE%/blocklist/");
> pref("extensions.blocklist.itemURL", "https://addons.mozilla.org/%LOCALE%/%APP%/blocked/%blockID%");
> 
> pref("extensions.update.autoUpdateDefault", true);
> 
>+// Disable add-ons installed into the shared user and system areas by default
nit: for clarity please change
s/shared user and system areas/shared user and shared system areas/

or mention that the application's extensions directory is not included in this.

>+// See the SCOPE constants in AddonManager.jsm for values to use here
>+pref("extensions.autoDisableScopes", 10);

>diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js
>--- a/browser/components/nsBrowserGlue.js
>+++ b/browser/components/nsBrowserGlue.js
>...
>@@ -394,16 +395,31 @@ BrowserGlue.prototype = {
>     if (this._isPlacesDatabaseLocked) {
>       this._showPlacesLockedNotificationBox();
>     }
> 
>     // If there are plugins installed that are outdated, and the user hasn't
>     // been warned about them yet, open the plugins update page.
>     if (Services.prefs.getBoolPref(PREF_PLUGINS_NOTIFYUSER))
>       this._showPluginUpdatePage();
>+
>+    // For any add-ons that were installed disabled and can be enabled offer
>+    // them to the user
>+    var win = this.getMostRecentBrowserWindow();
>+    var browser = win.gBrowser;
>+    var changedIDs = AddonManager.getStartupChanges(AddonManager.STARTUP_CHANGE_INSTALLED);
>+    AddonManager.getAddonsByIDs(changedIDs, function(aAddons) {
>+      aAddons.forEach(function(aAddon) {
>+        // If the add-on isn't user disabled or can't be enabled then skip it
>+        if (!aAddon.userDisabled || !(aAddon.permissions & AddonManager.PERM_CAN_ENABLE))
>+          return;
>+
>+        browser.selectedTab = browser.addTab("about:newaddon?id=" + aAddon.id);
>+      })
>+    });
Would it make sense to at some point reimplement this so selectedTab is only set for one tab? I noticed that _showPluginUpdatePage also sets selectedTab. Perhaps a followup bug if it does?

>   },
> 
>   _onQuitRequest: function BG__onQuitRequest(aCancelQuit, aQuitType) {
>     // If user has already dismissed quit request, then do nothing
>     if ((aCancelQuit instanceof Ci.nsISupportsPRBool) && aCancelQuit.data)
>       return;
> 
>     // There are several cases where we won't show a dialog here:
>diff --git a/toolkit/locales/en-US/chrome/mozapps/extensions/newaddon.dtd b/toolkit/locales/en-US/chrome/mozapps/extensions/newaddon.dtd
>new file mode 100644
>--- /dev/null
>+++ b/toolkit/locales/en-US/chrome/mozapps/extensions/newaddon.dtd
>@@ -0,0 +1,11 @@
>+<!ENTITY title           "Install Add-on">
>+<!ENTITY intro           "Another program on your computer would like to modify
>+                          &brandShortName; with the following add-on:">
>+<!ENTITY warning         "Install add-ons only from authors whom you trust.">
>+<!ENTITY allow           "Allow this installation">
>+<!ENTITY later           "You can always change your mind at any time by going
>+                          to the Add-ons Manager.">
>+<!ENTITY continue        "Continue">
>+<!ENTITY restartMessage  "You must restart &brandShortName; to finish installing this add-on.">
>+<!ENTITY restartButton   "Restart &brandShortName;">
>+<!ENTITY cancelButton    "Cancel">
>diff --git a/toolkit/locales/en-US/chrome/mozapps/extensions/newaddon.properties b/toolkit/locales/en-US/chrome/mozapps/extensions/newaddon.properties
>new file mode 100644
>--- /dev/null
>+++ b/toolkit/locales/en-US/chrome/mozapps/extensions/newaddon.properties
>@@ -0,0 +1,6 @@
>+#LOCALIZATION NOTE (name) %1$S is the add-on name, %2$S is the add-on version
>+name=%1$S %2$S
>+#LOCALIZATION NOTE (author) %S is the author of the add-on
>+author=By %S
>+#LOCALIZATION NOTE (location) %S is the path the add-on is installed in
>+location=Location: %S
Not sure I like that the strings all refer to allowing the add-on install when in reality this is about enabling and disabling... I'm leaving that to UX though.

I didn't review anything past this file since you asked for browser only.
Attachment #552570 - Flags: review?(robert.bugzilla) → review+
(In reply to Robert Strong [:rstrong] (do not email) from comment #124)
> This is sub-optimal since the user will have the option to make the decision
> regarding one add-on only to be presented with making the decision regarding
> a different add-on. I don't think it should block getting this landed but
> this behavior should be evaluated for improvement.

Kind of agree, however our expectation is this will for the most part only happen for single add-on installs between runs.

(In reply to Robert Strong [:rstrong] (do not email) from comment #125)
> >+    var win = this.getMostRecentBrowserWindow();
> >+    var browser = win.gBrowser;
> >+    var changedIDs = AddonManager.getStartupChanges(AddonManager.STARTUP_CHANGE_INSTALLED);
> >+    AddonManager.getAddonsByIDs(changedIDs, function(aAddons) {
> >+      aAddons.forEach(function(aAddon) {
> >+        // If the add-on isn't user disabled or can't be enabled then skip it
> >+        if (!aAddon.userDisabled || !(aAddon.permissions & AddonManager.PERM_CAN_ENABLE))
> >+          return;
> >+
> >+        browser.selectedTab = browser.addTab("about:newaddon?id=" + aAddon.id);
> >+      })
> >+    });
> Would it make sense to at some point reimplement this so selectedTab is only
> set for one tab? I noticed that _showPluginUpdatePage also sets selectedTab.
> Perhaps a followup bug if it does?

Could do, again the hope is that only one add-on at a time will actually be installed here so I don't think it is worth bothering with at this point.

Landed: http://hg.mozilla.org/mozilla-central/rev/eee41544cb84

Most of the pieces of this patch are automatically tested aside from the actual code that opens the tab on startup and the restart of the application. A manual check of this would be useful.
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Flags: in-litmus?
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
about:newaddon closes itself when entered in the locationbar or clicked from about:about. So you should add the nsIAboutModule::HIDE_FROM_ABOUTABOUT flag to nsAboutRedirector.cpp.
(In reply to Steffen Wilberg from comment #127)
> about:newaddon closes itself when entered in the locationbar or clicked from
> about:about. So you should add the nsIAboutModule::HIDE_FROM_ABOUTABOUT flag
> to nsAboutRedirector.cpp.

Please file a bug for that, and if you want to be even more helpful a patch!
Blocks: 678660
(In reply to Dave Townsend (:Mossop) from comment #126)
> Landed: http://hg.mozilla.org/mozilla-central/rev/eee41544cb84

All the .addon-control stuff in toolkit/themes/winstripe/mozapps/extensions/newaddon.css is dead code, right?
(In reply to Dão Gottwald [:dao] from comment #129)
> (In reply to Dave Townsend (:Mossop) from comment #126)
> > Landed: http://hg.mozilla.org/mozilla-central/rev/eee41544cb84
> 
> All the .addon-control stuff in
> toolkit/themes/winstripe/mozapps/extensions/newaddon.css is dead code, right?

Uhh yeah, I could have sworn I had removed that already.
(In reply to Dão Gottwald [:dao] from comment #129)
> (In reply to Dave Townsend (:Mossop) from comment #126)
> > Landed: http://hg.mozilla.org/mozilla-central/rev/eee41544cb84
> 
> All the .addon-control stuff in
> toolkit/themes/winstripe/mozapps/extensions/newaddon.css is dead code, right?

Backed these out on inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/36a5644e9d6f
If I read the code correctly, this doesn't handle several addons being added very nicely, does it?
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a1) Gecko/20110822 Firefox/9.0a1 SeaMonkey/2.6a1 ID:20110822003134

I saw this popup on L64 in nightlies of Fx9 (with one 3rd-party extension from my distro, plus several installed by me) and of Sm2.6 (showing the four built-in extensions, but two of them haven't yet got a compatible maxVersion) and AFAICT it worked as expected. I didn't run extensive tests though, so I'll let someone else VERIFY the fix in more detail.
Depends on: 684190
The steps from the description are up to date in order to verify this?
I have Microsoft .NET Framework 3.5 Service Pack 1. It's ok to install it and verify this enhancement?
(In reply to Maniac Vlad Florin (:vladmaniac) from comment #135)
> The steps from the description are up to date in order to verify this?
> I have Microsoft .NET Framework 3.5 Service Pack 1. It's ok to install it
> and verify this enhancement?

No, as far as i can tell; at least for now it only checks extensions, not plugins.
http://support.microsoft.com/kb/963707(In reply to Cork from comment #136)
> (In reply to Maniac Vlad Florin (:vladmaniac) from comment #135)
> 
> No, as far as i can tell; at least for now it only checks extensions, not
> plugins.

That was about the Firefox extension not the plugin that ships/shipped with the .NET runtime (.NET Framework Assistant).
Looks like Michael Verdi (CCed) had some issues when testing this, from IRC:

<verdi> no 3rd party addons are getting disabled. I installed the add-on compatibility reporter addon to make (maybe) see if it was just because they're not compatible with Fx 8 but that didn't seem to help http://people.mozilla.org/~mverdi/screenshots/holy-crap-20111010-175724.jpg
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Michael Verdi should file a new bug, blocking this one, with steps to reproduce.
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
(In reply to Dão Gottwald [:dao] from comment #139)
> Michael Verdi should file a new bug, blocking this one, with steps to
> reproduce.

I filed this bug https://bugzilla.mozilla.org/show_bug.cgi?id=693459 and then went on to try other bundled addons and they all got installed (this is what limi is referring to in comment 138).

I'll try again and file a bug with steps to reproduce.
Depends on: 693743
Depends on: 694267
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0

Verified fixed on Beta, Aurora and Nightly on Mac OS 10.6, Windows 7, Windows XP and Ubuntu 64.

The third party warning appears inside a new tab the first time Firefox is launched after a third party add-on installation with the design specified in http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/addOnLocalInstall-i2.png
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.