Closed Bug 596343 Opened 14 years ago Closed 13 years ago

Users should have exclusive control over selecting their add-ons

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

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

People

(Reporter: jboriss, Assigned: mossop)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [parity-ie])

Attachments

(16 files, 8 obsolete files)

41.46 KB, image/png
Details
702.79 KB, image/png
Details
197.65 KB, image/png
Details
391.20 KB, image/png
Details
388.98 KB, image/png
Details
53.82 KB, image/png
Details
83.22 KB, image/png
jboriss
: ui-review+
Details
53.46 KB, image/png
jboriss
: ui-review+
Details
124.60 KB, image/png
jboriss
: ui-review+
Details
104.27 KB, patch
Unfocused
: review+
Pike
: feedback-
Details | Diff | Splinter Review
104.06 KB, patch
mossop
: review+
dao
: review-
Pike
: feedback+
Details | Diff | Splinter Review
27.80 KB, image/jpeg
Details
1.16 KB, patch
Unfocused
: review+
Details | Diff | Splinter Review
83.55 KB, image/png
Details
29.09 KB, patch
Details | Diff | Splinter Review
15.22 KB, image/png
Details
When a user upgrades to Firefox 4.0 from any previous version, any currently-enabled plug-ins installed by a third party application should be automatically disabled.

This is in response performance drawbacks created by plug-ins that, in many cases, the user has not consented to install.  Some examples, such as eBay's Browser Highlighter, cause serious startup time delay.  Installing add-ons in this method is not necessary behavior for any application.

We're unable to remove these add-ons for Firefox 4.0, so instead they should be disabled when the user upgrades.  If the user wants to enable one of these add-ons, doing so only requires enabling the disabled add-on via the add-ons manager.
Keep in mind that many OEM's provide system installed extensions to add functionality for their hardware / software. For example, HP and Lenovo both provide fingerprint reader extensions.

Also, I highly suspect there are many more extensions that can be installed into the user profile that cause the same root problem of performance that is the driving reason to do this.

Since many if not the majority of the system installed extensions do not cause a performance problem I'd suggest at the very least only offering to disable them otherwise the user will be missing functionality they are expecting after upgrade.
This bug must include notification to users after startup so they know things have been disabled you should probably start a thread about it in the newsgroups and we probably need to reach out to the AV vendors so they know that this is coming.
Another thing I really don't like about this is that all system extensions will be disabled due to a couple that cause perf degradation (e.g. eBay's
Browser Highlighter from comment #1). If the goal is to improve perrformance / prevent add-ons from causing a performance issue (which this bug seems to be about) I think the proper way to address it would be to first reach out to the authors of those add-ons and if there isn't resolution to the issue to soft block the add-on instead of disabling all system add-ons especially since there have been profile add-ons that cause the same perf issue.
I'm not a fan of this approach at all. When we find out about extensions/plugins that impact adversely, we start the blocklist process, which includes outreach. The browser highlighter is an example where we did reach out, and got a pretty quick turnaround in addressing the problem and getting a new version out there.

I think this type of problem is better addressed through blocklisting, which has an established process for mitigating the effects of extensions that dramatically impact UX. We've worked with a number of addon developers to mitigate bad actors and, by and large, response has been excellent. I'd prefer we keep doing it that way.

In a number of cases user consent is obtained before installation, and making the decision to counter that on our users behalf doesn't strike me as the right thing to do. I am, however, all for a notification of some kind that outlines extensions we have flagged as something that adversely affects Fx perf/usability, and provides additional information of what the effects are, giving the user the ability to disable/update/uninstall to mitigate the effects.

I'd like to look at alternatives to what's proposed in comment #0 to better educate users and devs, rather than taking a blanket approach which may be harmful on a few levels.
I understand the concerns from comment 3 and comment 4, but the basic fact is: - we do not know if users asked for these plugins / add-ons to be installed, - in many cases we do know that they did not ask (ie: Skype, Norton), - in *all* cases we know it affects perf (any addon does)We want to shift to a system where third-party vendors have to ask the Firefox user's permission to install add-ons, which is consistent with our values and beliefs. The best way to do that is to have a link to the add-on install (which can be locally hosted or remotely hosted) which launches as part of their application installation.The alternative is to blocklist all of these add-ons *except* for the ones we approve, which actually feels less friendly to me than this mechanism by which we explain to users and allow them to re-enable the third-party software if they have decided that they want to use it.Yes, we should work with our partners on this, but that work will include making them understand that silent installation of software isn't really respectful of users.
We've *recommended* that third-party extensions are installed in this way. Did we implement the dialog box we've talked about "a new extension is available, do you want to enable it"? If so, can we just present this dialog on upgrade? If not, how is upgrade any different from a fresh install?
It's not any different, and yes, that's the sort of dialog box we're talking about, with a special case on the upgrading Firefox user path to clean out years worth of crufty, potentially unwanted installations.

That we gave people bad advice is not a great defence for a poor end experience for our users. We should recommend third-party extensions do something smarter.
(In reply to comment #7)
> It's not any different, and yes, that's the sort of dialog box we're talking
> about, with a special case on the upgrading Firefox user path to clean out
> years worth of crufty, potentially unwanted installations.

So this depends on bug 476430?

> That we gave people bad advice is not a great defence for a poor end experience
> for our users. We should recommend third-party extensions do something smarter.

But regardless of how smart they've been about informing users about installing their integration, this bug will still disable them.
So, I think the end-game we *want* is for third-party apps to install using the registry, and present a dialog on next startup "a new extension is available, do you want to enable it". Right?It's not clear to me what part of this is already implemented.
Emphatically disagee with comment #5, because in many cases we've identified the no consent as an issue, and the vendors have changed their behaviour as a result of our outreach (Skype and Norton are good examples of this). We outline methods for installing addons programatically on mdc, but don't speak to best practices there, and that's something which should probably be addressed there as well.

I totally agree that going permissions based is the way to go, and know we've discussed how we expose that to vendors to use. The issue, to me, is that we're making some assumptions that may or may not be valid, and this approach is a pretty dramatic shift from how we've operated in the past and I'm not convinced said assumptions are valid.

I definitely think this is something we need to address and push vendors towards (with pointy things, even). We also need to ensure the user education/experience piece is fully fleshed out so users understand what we're doing, why we're doing it, and how to re-enable/update/tell us where to go/whatever as part of the exp. I am deeply concerned that if this is rushed/included for a product release a couple months out, vendors won't have time to get updates out, and we risk making our users angry and burning some relationships.

All for something to give users what they need to make an informed decision and make their UX as great as it can be, just think the education, implementation, and impacts need to be worked out further. I'd also love AMO input here, too.
Moar VPs needed on this bug.
It's entirely possible that bug 476430 gets us most of the way there, as long as there's a way to trigger it en-masse for cases where we haven't already asked for user permission.

Kev: I think the assumption you're making is that it would be very difficult for users to re-enable these plugins. What we're talking about is: disabling them on the first Firefox 4 run, with UI that says "if you did want these things, go ahead and turn 'em on."
My assumption is that the UI will be simple for users to re-enable, and agree that we should be moving aggressively towards a positive-user-opt-in experience.  I don't think it's too onerous, or that it will have deleterious effects on add-ons that users agreed to install.  (Ones that are snuck in or cryptically named may suffer; I think that is a sign of success.)

We sure are close to feature freeze, though!
(In reply to comment #5)
> I understand the concerns from comment 3 and comment 4, but the basic fact is:
I'm fine with providing a notification and options to the user. I don't think it is a good idea to disable add-ons without notification for the perf reasons as stated in comment #0.
(In reply to comment #12)
> Kev: I think the assumption you're making is that it would be very difficult
> for users to re-enable these plugins. What we're talking about is: disabling
> them on the first Firefox 4 run, with UI that says "if you did want these
> things, go ahead and turn 'em on."

I think this over-simplifies things, because we're assuming that users will understand whatever notification and UI is being presented. I admit I am overly cautious by default, but I want to ensure it's clear to users what we're doing, why we're doing it, what effects it may have (which will be hard if we don't know what we're turning off), and how to flip things back if they realize something's missing. Using the AOM is second-nature to us, but I don't know if that is the case with our users (that said, I don't know it isn't, but experience with f&f tech support gives me pause).

I'm more in favor of notification, education, and giving the users options to make an informed choice. Making that choice for them for existing installs doesn't feel right, and I'd like for us to consider flagging add-ons installed outside the in-use profile with a "hey, did you know about these?" and give the users information and options to disable (and ideally expose the AOM and how to use it), instead of a disable and assumption that the user will understand how to re-enable. I think I'm showing my, um, experience and repeating myself, so I'll leave it there.
>I'd like for us to consider flagging add-ons installed
>outside the in-use profile with a "hey, did you know about these?" and give the
>users information and options to disable (and ideally expose the AOM and how to
>use it)

If the user is required to understand the existence of a problem that they may want to solve for themselves (and one that is not their fault), it's unfortunately very likely that they will ignore our attempts to help them.
blocking2.0: --- → ?
My biggest issue with these add-ons is that users can't uninstall software here, and there are tons of add-ons we can't update because of the install mechanism. (java console).  These add-ons are effectively orphaned and can only be upgraded via some third party installer.  I'd propose going even farther, and hiding completely add-ons which are more than 1 major version behind.  It's frustrating to not be able to remove software you installed without wiping your Fx profile.

Can we move these extensions into the profile directory?
(In reply to comment #17)
> My biggest issue with these add-ons is that users can't uninstall software
> here, and there are tons of add-ons we can't update because of the install
> mechanism. (java console).  These add-ons are effectively orphaned and can only
> be upgraded via some third party installer.  I'd propose going even farther,
> and hiding completely add-ons which are more than 1 major version behind.  It's
> frustrating to not be able to remove software you installed without wiping your
> Fx profile.
> 
> Can we move these extensions into the profile directory?
We did outreach as soon as we found out about the Java console. Has this continued to happen with new versions of the JRE? Also, wiping the Fx profile doesn't fix the Java Console issue since it is installed into the installation directory's extensions directory.

Regarding a possible solution for extensions installed into the installation directory's extensions directory we could only allow app approved extensions into this directory and Mossop and I have discussed implementing something similar to distribution bundles where an extension could be staged and installed only once from this staged directory into a profile and from that point only update via the add-ons manager update mechanism.
I think it only makes sense to do this in the first version that includes a fix for bug 476430, which will be after Firefox 4.
>I think it only makes sense to do this in the first version that includes a fix
>for bug 476430, which will be after Firefox 4.

We are expecting many users who have departed Firefox to come back and try out Firefox 4.  Unfortunately this means that if we only fix this issue in a later point release, we lose the significant opportunity of demonstrating to these users that Firefox 4 no longer has the issues that they were experiencing with Firefox 3.  For instance, it isn't clear how many users left Firefox for IE or Chrome after installing Skype and experiencing a 30 second start up time, but given the reach of Skype's distribution, the number is likely sizeable, and a number of these users may want to come back to Firefox 4. (this is perhaps a bad example in that I believe we finially blocked that extension, but you could easily replace skype with any other widely distributed software appliation that has been injecting performance impacting add-ons into Firefox 3).
Agreed that you could "replace skype with any other widely distributed software appliation that has been injecting performance impacting add-ons into Firefox 3". It is important to also consider "any other widely distributed software appliation" that has been injecting features into Firefox such as Lenovo or HP in regards to their fingerprint reader / password manager feature integration into Firefox. These are much better known quantities than the current empty list of add-ons that are installed at the system level that cause an impact on performance with Firefox.
OS: Mac OS X → All
Hardware: x86 → All
blocking2.0: ? → -
(In reply to comment #21)
> It is important to also consider "any other widely distributed software
> appliation" that has been injecting features into Firefox such as Lenovo or HP
> in regards to their fingerprint reader / password manager feature integration
> into Firefox.

Which is why we tell users they have been disabled, and give them the option to re-enable. This is very much one of our top performance and startup UX issues, and we need to do something to fix this problem.
If it is purely due to add-ons causing perf issues then it should be done for all add-ons since we do know of non system add-ons that cause perf issues while we know of no system level add-ons that do currently though I highly doubt that would be what you want. Also, I can't help but question this being a top performance issue when the only cases we know of have already been addressed.
(In reply to comment #22)
> Which is why we tell users they have been disabled, and give them the option to
> re-enable. This is very much one of our top performance and startup UX issues,
> and we need to do something to fix this problem.

I would like to see the data that backs this up. If there are specific items that are still causing problems, we should blocklist them now. Is this something we think is true, or know is true? If it's the latter, then we should jump on it ASAP, as we've got the tools to do so. If it's the former, we really need to do some digging to back this kind of decision up (my opinion only, of course ;) ).

I think we should use the mechanisms we have in place, that were specifically designed to mitigate this type of problem. Comment #16 gives me even more pause, for if we believe the users don't pay attention if we try to help them disable things, I'm not sure why it wouldn't be the same with the opposite use case (re-enabling).

Name the worst transgressors, and I'll do whatever I can to mitigate the problems they cause.
It's definitely the case that we lack the data we need to make a fully informed decision.  However, I am worried that we didn't act on the ebay browser highlighter creating a 30 second start up time for a massive population of users until I personally randomly encountered someone in the real world who was effected.  The percent of the world using Firefox, and the economics of crapware all indicate that forced in third party extensions are a serious problem, even if it's a problem that we haven't been able to fully explore yet.

The goal of this bug is to give users control, and to provide them with a positive experience when they first try out Firefox 4.  The only way for us to currently guarantee that Firefox 4 works well is to limit the user's add-ons to the ones that they actually indicated that they wanted.
Now that b7 has shipped and feature freeze is behind us, is this something that we can realistically do?

If so, I would be willing to do it. This seems extremely important.
Re-nominating as blocker after today's Firefox meeting discussion.

More background info and reasoning stated by Faaborg & myself in the comments preceding this.
blocking2.0: - → ?
We do the right thing for users. If we convince ourselves that this is a big performance problem (which I believe to be true on Windows) and that many software vendors are abusing this mechanism to install plugins (which I know to be true on Windows) then we can bend whatever rules we have made in order to make it happen.

Bug 476430 seems to be a precursor for this, since there's no sense in disabling and then allowing them all back in again without warning. However, I'd like to do something on first-run that requires users to check off the various third-party-installed add-ons that they want to continue to work with Firefox 4. I believe that this can be done in a way that will not be offensive to third-party vendors, and will be very pro-user.

I think we should re-consider blocking, here.
>requires users to check off the various third-party-installed add-ons

If users take the shortest path forward, or if users don't understand what we are asking (both of which I believe will be the case), then we haven't ultimately helped them with this approach.  This is especially true if correcting the problem means that they have to click many individual check boxes, compounded with them being hesitant to make modifications to something that they do not understand.  I believe that we will only start to solve user's problems on Windows if we make this interface opt-in instead of opt-out.
Although there are numerous people on this bug with greater experience and relevance to this topic than me, I'd like to propose an analogy for what this will make the average user experience:

Dear Driver Of This Vehicle,

We have found that some people have been putting heavy objects in the trunks of some cars, and this has been making those cars drive very slowly. So, we have emptied your entire trunk and put its contents into a bag somewhere around here (include a sign pointing to bag). If you still wish to have these items in your trunk, please simply pick them out of the bag and place them back in your trunk. Enjoy your new, faster car!

Sincerely,

The Manufacturer
Car analogies are not appropriate for this, nor do they add anything of value. Please stay on topic.
I wasn't in the meeting where it was decided to renominate this so perhaps I'm missing some new context but nothing I've seen in the latest comments seems to be new information though so really my thoughts on this feature are unchanged. As faaborg says the only way you can get the results you seem to be looking for is to make it so the majority of users disable their third-party installed extensions (whether blindly or otherwise) thus basically screwing over the third-party developers who have followed the best possible methods we can give them at this time for integrating with Firefox. True some people are abusing the system but really I don't have a good idea of how many which makes it difficult to call on whether it is worth burning the good guys to take down the bad and really until we give third-parties an alternative (bug 476430) I don't think we should be putting energy into this one-time win.

I don't believe that this bug should block Gecko 2.0 and frankly I wouldn't like to see the feature as suggested implemented in the platform at all. However if the Firefox product drivers and UX team decide that it must block Firefox 4.0 despite the objections from this bug then that is obviously their call to make. In that case this can and I think should be implemented in the Firefox application code and so I'm moving this to the Firefox product for further evaluation.
Component: Add-ons Manager → General
Product: Toolkit → Firefox
QA Contact: add-ons.manager → general
Version: unspecified → Trunk
Dave: I encourage you to read comment 28, which I think you might have missed or misinterpreted. I don't think we can do this in isolation; it must be in combination with bug 476430 such that in the future we can be sure that users have agreed to allow third-party vendors to install add-ons. I'd also like a future where we can detect when those add-ons have a serious deleterious effect on performance, but know we can't reasonably get that in time for Firefox 4 (if I'm mistaken there, or if we think some of the code from bug 593743 can help us, please let me know!)

I'm more sensitive than I perhaps seem in these comments to the plight of third-party vendors who have followed the appropriate steps to integrate with Firefox. So let's build a better mechanism (bug 476430) and a one time method for helping users who might have a bunch of third party add-ons installed without their knowledge (this bug). I don't think "we haven't had a better way than this in the past, and so we shouldn't do anything to help users" is a good answer.

I'd like to co-ordinate with Kev before marking this as a blocker, and will do so offline.

Alex: the mechanism I'm proposing would be opt-in instead of opt-out for third-party installed add-ons, and opt-out for user-installed add-ons. I share your concerns about a lack of understanding, but in the absence of a stronger proposal for what to do that allows easy user control over these automatic actions, I think we should proceed.
(In reply to comment #33)
> Alex: the mechanism I'm proposing would be opt-in instead of opt-out for
> third-party installed add-ons, and opt-out for user-installed add-ons. I share
> your concerns about a lack of understanding, but in the absence of a stronger
> proposal for what to do that allows easy user control over these automatic
> actions, I think we should proceed.

I'm pretty sure this is exactly what we wanted, opt-in for third-party ("slipstreamed") add-ons, opt-out for the ones the user has actually installed. So I think there is agreement here, and just wanted to explicitly state it. :)

(We did start out with a position to be everything be opt-in as a starting point, months ago, but we quickly moved away from that — just for the record)
> Alex: the mechanism I'm proposing would be opt-in instead of opt-out for
> third-party installed add-ons, and opt-out for user-installed add-ons.

yeah we all agree on that, anything that is under our control through AMO we can profile and check for performance problems.  Also user installed add-ons outside of AMO are likely to be rather rare, and we can in those cases we can logically assume that the users know what they are doing.
I'd like to summarize the discussion so far in light of blocking nomination.

Currently, the add-ons a user has installed can be grouped into two categories: user-installed and third-party installed.

User installed add-ons:
1. All affect performance
2. Are assumed to be purposely installed by the user
3. Can be disabled and removed

Third-party installed add-ons:
1. All affect performance
2. May or may not be purposely installed by the user
3. Can be disabled but not removed

While we currently support developers installing add-ons via third party, there's been general unease with the practice.  This is especially because these add-ons not be purposely installed and can't be removed.  While Mozilla would like to promote Firefox integration with applications, we should only support it to the extent that the user is in control.  It's when applications promote themselves to the detriment of the user's experience and Firefox's performance that things get hairy.

To optimize for Firefox, we'd either prevent third-party add-on installation entirely or ask the user to opt in once an application has asked to install an add-on.

Right now, we're optimizing for applications over Firefox and its users by allowing third parties to invisibly install add-ons that can never be removed.

Since optimizing for applications is agreed to suck, bug 476430 proposes warning users when third-party applications want to install an add-on and asking them to opt in.  This is widely agreed to be Totally Reasonable, as it both allows applications to integrate with Firefox without requiring the user to separately install an .xpi while giving users executive control over what's being installed.

The question in this bug is what to do with all of the user's currently installed third-party add-ons.  What many are proposing here is asking the user (when they upgrade) which third-party add-ons they would like to use.  It's essentially giving the request for permission that we should have done for each add-on automatically.  Asking as a dedicated step of upgrading is even more direct and noticeable than asking in an arrow-notification, as we may do in bug 476430.  In the best case, the user sees a list of add-ons they never wanted, and their Firefox is faster once they are disabled.  In the worst case, a user misreads or skips this step, and an add-on they use and want is disabled.  Just disabled, not removed: recovering the add-on's functionality is only as difficult as clicking "enable" in the add-ons manager.

This request for permission, while necessary, need only be done once as long as bug 476430 eventually is fixed.  We should always be asking the user's permission to install a Firefox add-on.

(In reply to comment #32)
> As faaborg says the only way you can get the results you seem to be looking for
> is to make it so the majority of users disable their third-party installed
> extensions (whether blindly or otherwise) thus basically screwing over the
> third-party developers who have followed the best possible methods we can give
> them at this time for integrating with Firefox.

We allowed far too open a method for integrating with Firefox by allowing installation without user permission.  You're entirely right: it's not the fault of add-on developers.  However, the fact that we may have made a mistake here in making the process too permissive doesn't mean we should be afraid to correct it now.  Simply the fact that the user can't remove a third-party add-on indicates the balance may not be right here.
>perhaps I'm missing some new context but nothing I've seen in the latest comments
>seems to be new information

For full context, here are the three recent events that have contributed to my thinking on this:

1) The JS team is doing an awesome job and everyone (us, the tech press) are hailing the speed of Firefox 4.  Hopefully users who have switched away from Firefox for performance reasons are going to see those reports and come back to see if we solved the problems they were facing.  Unfortunately, the issues they are facing and the benchmarks may be entirely unrelated.

2) anecdotal conversation overheard between two people outside of Mozilla: "Great, today Firefox is refusing to search google, and now only directs searches to some shady site, how am I supposed to fix that?" "you can download Chrome in about 10 seconds." ... "ok, done."

3) anecdotal conversation with another Mozilla employee (paraphrasing) "my landlord's Firefox had an add-on from a greeting card company.  It's getting worse. This is why people switched to us from IE6, and now it's happening to us."

>makes it difficult to call on whether it is worth burning the good guys
>to take down the bad and really until we give third-parties an alternative

I don't think it is worth trying to judge if the third parties happen to be good or bad.  We simply need to ensure:

-Users are starting out with the product that we created
-Users have exclusive control over making modifications
I don't think that characterizing it as "screwing" add-on developers is appropriate.  Add-on developers whose users know and want their add-ons will have a very simple remedy to recommend, if the users don't find it themselves.  If they aren't sure that users will bother to turn the add-on back on, then I don't think we should take their complaints too personally.

Users who want the add-on to be installed will have a clear way to do it, and principled objections to that are few and far between.  That we are doing it so late in the cycle will make it harder for us to produce well-structured messaging to help add-on developers update their documentation for users, installer instructions, etc., but I don't think that outweighs the well-demonstrated frequency of users running code in their browser that they don't know about or understand to be different from Firefox.
Flags: in-litmus?
(In reply to comment #38)
> Add-on developers whose users know and want their add-ons will
> have a very simple remedy to recommend, if the users don't find it themselves. 
> If they aren't sure that users will bother to turn the add-on back on, then I
> don't think we should take their complaints too personally.

Also, when bug 476430 lands and notifies users about third-party installed add-ons, all these developers will need to do is push another Firefox install via their application and hope users agree to give permission.  If developers don't want to, they need not even change their code nor documentation.  If developers are worried users won't give permission, they're precisely the case that we're trying to guard against.
I am making the decision that this is a priority issue for Firefox 4. I've spoken with many of the stakeholders privately, and believe that I fully understand their concerns and they do mine as well. For me it's less about issues of potential performance and stability (though those are indeed concerns) and more about enabling proper user choice.

I've marked bug 476430 as blocking this one, and this will block the final release of Firefox 4. In order to execute on this, we'll need to provide screen flows for the user experience so that we can start communicating with third-party plugin providers and helping them understand what the experience will be. Limi, I'll need the team's help with that.

Johnath: sorry that this appears to be popping up late in the cycle. It's my fault for not tracking it better, but I believe that it was an agreed-upon pillar of the Firefox 4 plan many moons ago to try and clean up profiles which contained non-user installed software.

Axel: heads up, there will be strings here.
blocking2.0: ? → betaN+
Depends on: 476430
Keywords: uiwanted
Whiteboard: [strings]
The attached wireframe shows what a third-party add-on permission screen could look like as part of the Firefox software update process.  It's about as simple as the screen could get: a brief explanation of how the add-ons were installed and that any the user wants should be checked. 

There really isn't a need to give a more complex description of the reason for the screen or potential problems associated with the add-ons.  The fact that users should install add-ons only from authors they trust, which is the warning we give on standard add-on installation, is unneeded, because applications rather than authors are the origin of the add-ons.  We could say that the user should only keep add-ons from applications they trust, but this clouds the issue by unnecessarily implying that the user may already be using untrustworthy applications  (without any ability to say which are untrustworthy nor why).  Better to keep it simple, and at the maximum provide a link to more information.

(In reply to comment #17)
> I'd propose going even farther,
> and hiding completely add-ons which are more than 1 major version behind.  It's
> frustrating to not be able to remove software you installed without wiping your
> Fx profile.

Not listing such add-ons in this screen could be a very positive move.  These certainly aren't the third-party vendors we're worried about offending.
I'm going to start working on designing the interface changes we need for this.  To start with, here is a review of the current major update process that users go through:

http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/currentUpgradeFlow-i2.png

Here is a potential add-on selection screen, and some discussion of where we could insert this step into the process:

http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/userSelection-i1.png

I propose that we replace the compatibility wizard UI as opposed to adding additional steps.  Inserting this into the update itself is ideal, but very likely out of scope for Firefox 4 since this would require changes to 3.x prior to 4's release.
Keep in mind that this would need to apply to when a user installs (e.g. installer, dmg, tar) as well as when a user updates through app update which the add-on compatibility wizard will provide as is. Also, I would very much prefer simplifying and minimizing the app update ui in future versions (e.g. simple widget in the about page, getting rid of the billboard and instead opening a web page, etc.).
Should have also stated that the suggestion of "getting rid of the billboard and instead opening a web page" is only a suggestion and the implementation would need to be worked out.
Summary: Upgrading to Firefox 4.0 should disable all third-party installed extensions → Users should have exclusive control over selecting their add-ons
>Keep in mind that this would need to apply to when a user installs (e.g.
>installer, dmg, tar)

If we replace the compatibility wizard with this new UI, then it would continue to follow the installer (or launch after the user launches Firefox copied out of a dmg).  But yeah, in the future If we place this in the app update UI, then we will also need to place it directly into the installer as well.
The windows setup, mac dmg, and linux tar's have absolutely no understanding of our extension and profile system and teaching them about our extension system would be a very big PITA. If we were to go that route I would strongly suggest we abandon using 3rd party installers and go back to building our own based on Mozilla since it would then understand our extension and profile system as well as make it possible to add the non-installer features that are periodically brought up.
>But yeah, in the future If we place this in the app update UI, then
>we will also need to place it directly into the installer as well.

I guess I should revise that, if we place this in the app update UI, then we would at least need to keep around the compatibility wizard style approach to directly follow the installer or launching after being pulled out of the dmg/tar.  Either way, future work.
(In reply to comment #42)
> Here is a potential add-on selection screen, and some discussion of where we
> could insert this step into the process:
> 
> http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/userSelection-i1.png

From an outsider's POV, looks awesome, but couple of things that are unclear: (And given how big a role this dialog box plays, hope you can excuse my interference!)

For cases where the enable/disable checkbox is greyed out (eg: addon has no version that is compatible or AMO down/user offline), it's not clear whether the addon will still be enabled or not in the addons manager? 

ie: is the addon 'enabled' and awaiting updates that make it compatible before it will run, or will it remain disabled, even if updates later become available? 

It just seems like currently, not only is the default behaviour unclear (final enable/disable state for that addon), but there is no way for the user to override it - eg: the user can't make a 3rd party addon enabled or alternatively disable a "installed by them" addon.

In order to avoid this problem, perhaps it might be best to not combine the process of enabling/disabling addons with the compatibility/updates check stage - ie: not have everything on the same page of the wizard/workflow. Currently there are an awful lot of options on that dialog, which given how important the enable/disable question is, might just mean a lot of people click done without reading.

How about:

- 1st page of dialog: Lists the addons like the current mockup, except no mention of compatibility/updates, just: "Addon name", "Installed by" and "enable" checkbox. (Nothing is greyed out).

- Subsequent pages deal with compatibility/updates - and only need worry about the addons that were chosen as enabled in the previous step.

Anyway a mockup is probably easier to follow, so if you can excuse my Photoshop butchering, I've made one for some ideas here:

http://oi54.tinypic.com/n2y0qx.jpg

Hope you don't mind my comments/suggestions...
Feels like the user-installed add-ons should take precedence. We might also want to use headers to make things clearer, like:

------------------

                       Your Add-Ons

Get rid of Add-Ons you don't use anymore, and get updates for the ones you love and want to keep!

// Installed by You

GreaseMonkey                 (o) Keep   ( ) Disable
FooBarBazTastic              (o) Keep   ( ) Disable

// Installed by Other Applications

Skype Toolbar                ( ) Keep   (o) Disable
Digsby Stuff                 ( ) Keep   (o) Disable

You can always re-enable Add-Ons through the Add-Ons Manager if you change your mind later.

------------------

I don't think there's a lot of value to indicating if Add-Ons are compatible or not as at this point, there's nothing a user can do about it. We might want to do a summary screen like this:

------------------

Add-Ons you kept

  GreaseMonkey
  FooBarBazTastic
  Skype Toolbar

Add-Ons you disabled

  Digsby Stuff

Add-Ons that will be enabled once the author releases a version that is compatible with Firefox 4

  NotYetCompatible

If you want to add, remove, enable or disable Add-Ons, you can always do so by going to the Add-Ons Manager available in the Tools Menu

------------------
>ie: is the addon 'enabled' and awaiting updates that make it compatible before
>it will run, or will it remain disabled, even if updates later become
>available? 

yeah, I agree we need to solve this with the interface.  I don't think handling the confusion with an extra step necessarily helps though, since in that situation the user may be confused because we intentionally obfuscated the information by deferring it to later.  They will eventually understand everything, but may start with thinking "wait, where did all the add-ons I actually installed myself go?"


>which given how important the
>enable/disable question is, might just mean a lot of people click done without
>reading.

Breaking this apart into separate steps may actually accelerate users that are hitting a whatever button as many times as they need to in order to complete the upgrade flow.  But even with a single step, we do expect that a large number of users are going to ignore the message (just as they ignored a lot of checked by default options in other installers that added all of these extensions in the first place).  So while we want them to be informed, we are also aware that the the default set of choices will be accepted by a large number of users who aren't interested in directly managing this themselves.
>(o) Keep   ( ) Disable

Yeah, I think framing this around "keep" or not makes the most sense, so that we can disambiguate over if Firefox will re-enable once an update is found.  I'll post a new revision of the mockups later.

The other issue I was worried about was that some users will think that we are offering these (even though we aren't, they are already installed).  So "keep" helps a bit in trying to prevent that misunderstanding, and having users upset that we are now pushing the _____ toolbar in our own installer.
This is a very nice looking UI and I'd really like to see it replace the existing compatibility UI though there are of course some comments to make:

As some of the previous comments have mentioned it seems like we are conveying too much information here for the average user to really make a reasonable choice with this effectively single-page dialog. I think it becomes easier if we ignore the optional update bit and just automatically update when there is an update available (and the user has auto-updates on) or required (with some string like "an update will be installed"). The vast majority of users will have auto-updates enabled anyway so this will still get lots of people updated immediately.

I wonder if it might be worth a little more descriptive text at the top? I was surprised there was no explanation of what was going on.

There are a couple of technical problems that would make it troublesome to implement this as it stands:

We don't have small icons for extensions. We have a normally 32x32 pixel icon however we've recommended add-on authors increase that to 48x48 for Firefox 4.0. Squeezing that down to 16x6 is going to look blurry at best, likely nasty and unrecognisable.

I'm assuming that you want the central part of the UI to scroll leaving the headings at the top? I think the only way we can do this in XUL is with a tree which imposes a few restrictions onto what we can display. In fact all we get are simple columns with either checkboxes, text labels or progress bars. One of the text boxes can have an icon. This pretty much works out for this UI except I'm not sure we can do the line between the types, and the greyed out message for each add-on would have to be a separate column. If you switched to radio boxes as suggested above then we'd hit more problems.

There are also some additional things to think about:

As well as user installed and external app installed there can also be application bundled add-ons.

We need to consider what to do about errors that happen during the update part of the UI and whether to expose them to the user or not.
Application bundled add-ons from BYOB or a "companion" browser should be considered user-installed.
(In reply to comment #53)
> Application bundled add-ons from BYOB or a "companion" browser should be
> considered user-installed.

Let me know how we need to suss that out. Current BYOB-included extensions get installed to the user profile, so that shouldn't be an issue. Companion and legacy partner builds included global extensions that were installed to <AppDir>/extensions. It's a reasonably small list, but I know that some third-party extensions get dropped in there as well.
(In reply to comment #52)
> As some of the previous comments have mentioned it seems like we are conveying
> too much information here

I met with Faaborg earlier today to iterate on the proposal, and there's a simplified version coming. Part of it is also that the mock-up covered all the edge cases without showing how the common case would look like, so it looks overly complicated. The new mock-up should show the variations we have (one common, one that summarizes the edge cases), as well as removing several of the unnecessary/obvious labels. Hopefully that will make it a bit clearer.

> We don't have small icons for extensions. We have a normally 32x32 pixel icon
> however we've recommended add-on authors increase that to 48x48 for Firefox
> 4.0. Squeezing that down to 16x6 is going to look blurry at best, likely nasty
> and unrecognisable.

I don't think showing this list without icons would make it less useful. Most add-on icons are of pretty questionable legibility at small sizes anyway, so I think the name of the extension should be sufficient.
>I wonder if it might be worth a little more descriptive text at the top? I was
>surprised there was no explanation of what was going on.

As many people have noted, this is already a pretty complex presentation.  I also couldn't think of anything that provided much value over just a wordier version of "Select your Add-ons" (in the list below you can select which add-ons you would like to continue to use with Firefox 4, etc.).  We could attempt to explain why we unchecked the third party ones, but that's also a complicated topic.

>We don't have small icons for extensions.

Ok, I'll update the mockup to reflect that.  As Limi notes, not a big deal.

>I'm assuming that you want the central part of the UI to scroll leaving the
>headings at the top?

yes, also the toolbar with the controls to keep or disable all.  I'll add a scroll bar to the mockup so that it's clear.

>We need to consider what to do about errors that happen during the update part
>of the UI and whether to expose them to the user or not.

Sure, can you provide a list of potential errors?  We'll likely suppress most of them.
(In reply to comment #56)
> >We need to consider what to do about errors that happen during the update part
> >of the UI and whether to expose them to the user or not.
> 
> Sure, can you provide a list of potential errors?  We'll likely suppress most
> of them.

Pretty much any of the errors while attempting to download/extract. Could be a network timeout, could be a corrupt file, I suspect we don't need to go into details in the UI but do we at least want to say that an update was attempted and failed, particularly in the case where an update was necessary to make the add-on compatible?
Here is a new version based on the comments above:

http://people.mozilla.com/~faaborg/files/firefox4Mockups/userAddonSelection/userSelection-i2.png

This doesn't include error states yet, still need to add those.
>Feels like the user-installed add-ons should take precedence.

The potential downside to listing them first is that if there are enough of them, the third party extensions will be pushed below the fold, and users may click done not realizing that we are in fact disabling a number of extensions.

Which reminds me that I forgot the scroll bars in i2...
I haven't thought of a better layout but it seems a tad off having the control to change the action separated from the text describing the action as follows:

 Keep |   | Name           | Action
------+--------------------+------------------
 [ ]  | # | Add-on name    | will be disabled


I suspect that many people will just see the word Keep / not notice that Action stated will be disabled when Keep is not checked and expect the add-on to be uninstalled. I don't think there is a better term but wanted to call this out.
Alex: I like the addition of Keep All / Disable All buttons, but agree with comment 61. Feels important to have a layout closer to what I proposed in comment 49

Regarding the concern about listing user-installed add-ons first, the design could be such that the scrollbars only apply to each section (installed by you / installed by other applications) if the lists are indeed that long.
Whiteboard: [strings] → [strings][target-betaN]
blocking2.0: betaN+ → -
Whiteboard: [strings][target-betaN] → [strings][target-betaN][d?]
Based on implementation time estimates and QA requirements, we've decided that we can't wait for as long as it would take to get this done right. Work can continue, as we do want to do this for a future version of Firefox, to be sure, but for Firefox 4 we'll have to get a list of pathological cases and use the blocklist, instead.
Whiteboard: [strings][target-betaN][d?] → [strings]
I think actually IE 9 has a good approach to this.  It directly measures add-on startup performance and they have the "popup of shame" that will show up if the add-ons load too slow which will offer to disable the slow add-ons for you.

It may be harder to actually measure given the way firefox extensions work, but it might be possible.
(In reply to comment #64)
> I think actually IE 9 has a good approach to this.  It directly measures add-on
> startup performance and they have the "popup of shame" that will show up if the
> add-ons load too slow which will offer to disable the slow add-ons for you.
> 
> It may be harder to actually measure given the way firefox extensions work, but
> it might be possible.

I filed an enhancement request for this very thing (https://bugzilla.mozilla.org/show_bug.cgi?id=597282) back in September. Up to 11 votes so far, hopefully it gains traction.
Blocks: 502127
Assignee: nobody → dtownsend
Whiteboard: [strings]
Dunno where to post about it:
Obvious improvement for "Select your add-ons" window mockup described here:
http://areweprettyyet.com/5/userAddOnSelection/

Instead of column "Installed by" make groups in list with names like "Installed by [source name]".
It will give more space for add-on names and will reduce amount of visual garbage on screen.
(In reply to comment #66)
> Dunno where to post about it:
> Obvious improvement for "Select your add-ons" window mockup described here:
> http://areweprettyyet.com/5/userAddOnSelection/
> 
> Instead of column "Installed by" make groups in list with names like "Installed
> by [source name]".
> It will give more space for add-on names and will reduce amount of visual
> garbage on screen.

We don't have any way to determine that information though.
Is there any guard against 3rd party add-ons trying to reinstall themselves on every restart?
(In reply to comment #68)
> Is there any guard against 3rd party add-ons trying to reinstall themselves on
> every restart?

This wouldn't have any effect. They would have to uninstall themselves, start and exit Firefox and then install themselves again. If people start doing things like that then we'd start to talk about blocklisting pretty quickly.
> We don't have any way to determine that information though.
No, I'm not talking about detecting exact source. I just talking about two groups: installed by third-party and installed by you.
On current mockup we have separate column "installed by" and for each extension there is "third-party" or "you". I propose to group extensions by this column (actually they already grouped on mockup) and add headers to each group.
we need to make sure that "third party" is on screen even as the user scrolls (sadly some users are going to have more than one screen's worth of these things), so that we can express:

"we are disabling all of the third party installed extensions"

instead of it looking like (but not actually being)

"we are disabling all of your extensions"

the later would obvious cause a lot of excitement and fear in our extension development community.  So the interface is redundant and a bit cluttered, but for the purpose of constant clarity.
Depends on: 656125
We discussed this extensively today in the Add-ons Manager meeting and have a proposed revision that will address my and Kev's concerns about the flow.

We'd like a confirmation step to take place after the user hits 'Next' on this dialog that confirms what's going to happen if any add-ons are to be disabled. Something like:

----------------------------------------------------------------
The following add-ons will be disabled:
* Add-on 1
* Add-on 2

You can enable them at any time from the Add-ons Manager.
[Go Back] [OK]
----------------------------------------------------------------

Additionally, we'd like more room for explanation in the first screen to say why this dialog is being shown and what users should do. This dialog will only be seen once, so there's no need to rush everyone through it. Something like:

"Make Firefox even faster by disabling add-ons you no longer use. Add-ons already installed by third parties will be disabled automatically unless you select them below. _Learn_more_"
Here is an updated flow that has the changes requested by Justin and Kev.
I believe it should be done in one as "one window". In the proposal are to many steps and different window types, it will confuse the inexperienced user. 

My proposal: 

1. Opens the window with all add-ons listed. Progress bars appear for every single add-on listed (, as option there may be also a overall progress bar on top of the window)

2. In the same window are the compatibility icons shown. Two buttons: disable / delete are for action

3. The last step is "Updating add-ons"

I attach sceenshots, sorry I'm not a designer. :)
Attached image My propasal
I'd like there are bigger fonts/icons used.
I really like the proposal made by Eugene Savitsky (comments 74 & 75)

Except that having both Disable & Remove choices (in the 2nd step) may add unnecessary complexity and confusion.

Maybe that Mozilla should choose to display only one (Disable)
In a number of cases, we wouldn't be able to remove third-party addons, as they're stored under appdirs or other directories that we wouldn't have access to (which is part of the issue being addressed). Disabling is the option we have.

As far as the second screen, we want a summary to be presented to the user, so they have a confirmation before the changes are made. Users familiar with enabling and disabling addons aren't the target in this case, it's the users who aren't, and we want to make sure there are some clear signs for what's happened if they lose UI and/or functions they're used to having.
In our online shop experience the check boxes makes user be scared. The UI should be as clean and simple as downloading FF is: big font, easy to understand and sexy. The given proposal looks like made by programmers for programmers like the UI was back in 90th. IMHO.
(In reply to comment #78)
> In our online shop experience the check boxes makes user be scared.

This part seems like a fair critique.

What if attachment 541298 [details] presented the summary _first_, and only displayed the panel with the full details and checkboxes if the user clicks some button to change their choices?
Even look at the add-ons page in FF, actually that is the design we need. Users had seen this (most of them?), they are used to it. It has the "Remove" and "Disable" buttons and the "Undo" appears after deleting some add-on. For me it has all user needs.
Flow for add-on selection, now with delicious OSX flavor
Comment on attachment 542396 [details]
Updated flow for add-on selection (OSX)

overall looks great, three small nits:

-for some reason italics aren't used on OS X (when in rome and all that)
-need to add in the "keep all" and "disable all" buttons
-this is a good candidate for the bottom unified bar appearance we sometimes use on OS X.  Examples of this currently in use are the major update billboard, and the download manager (clear all button)
(In reply to comment #82)
> Comment on attachment 542396 [details]
> Updated flow for add-on selection (OSX)
> 
> overall looks great, three small nits:
> 
> -for some reason italics aren't used on OS X (when in rome and all that)

Removed

> -need to add in the "keep all" and "disable all" buttons

I spoke to Boriss about this (not know what they should look like on OSX was my whole reason for asking for some OSX mocks) and she was going to talk to you about removing them from Windows too as they aren't necessary and would look out of place on OSX. I don't really care either way but I need mockups to know how to present them if you want them there

> -this is a good candidate for the bottom unified bar appearance we sometimes
> use on OS X.  Examples of this currently in use are the major update
> billboard, and the download manager (clear all button)

Using this already
(In reply to comment #83)
> (In reply to comment #82)
> > Comment on attachment 542396 [details]

> I spoke to Boriss about this (not know what they should look like on OSX was
> my whole reason for asking for some OSX mocks) and she was going to talk to
> you about removing them from Windows too as they aren't necessary and would
> look out of place on OSX. I don't really care either way but I need mockups
> to know how to present them if you want them there

I talked to faaborg - we're going to remove the "keep all" and "disable all" buttons on both OSX and on Windows.

I'll add an OSX version without italics.
same but without italics
Attached patch patch rev 1 (obsolete) — Splinter Review
This is the implementation and tests. Going to spin screenshots past UX so there might be some pixel tinkering but I'd like to get the review for this underway now
Attachment #543182 - Flags: review?(bmcbride)
(In reply to comment #86)
> Created attachment 543182 [details] [diff] [review] [review]
> patch rev 1
> 
> This is the implementation and tests. Going to spin screenshots past UX so
> there might be some pixel tinkering but I'd like to get the review for this
> underway now

Dave, can make a try build happen with this? Is this behind the kind of flag that would make it easily disabled and thus a potential candidate for 7 (we're 48 hours from migration) or is this an early 8 landing?
(In reply to comment #87)
> (In reply to comment #86)
> > Created attachment 543182 [details] [diff] [review] [review] [review]
> > patch rev 1
> > 
> > This is the implementation and tests. Going to spin screenshots past UX so
> > there might be some pixel tinkering but I'd like to get the review for this
> > underway now
> 
> Dave, can make a try build happen with this? Is this behind the kind of flag
> that would make it easily disabled and thus a potential candidate for 7
> (we're 48 hours from migration) or is this an early 8 landing?

We don't want to land this without bug 476430 which has enough remaining issues that these are going to be early 8 landings
Comment on attachment 543182 [details] [diff] [review]
patch rev 1

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

Nothing major to note - just a bunch of small tweaks.

General notes:
* Feels like addons should be also ordered by current enable/disable state. If there are a bunch of already user disabled addons, they add a lot of noise to the list, but little benefit - maybe put them right at the bottom? (and/or ask to uninstall them?).
* It's a little disconcerting when most rows have a blank space in the "action" column.
* The column headers need tweaking on Win7 - the borders look too dark.
* Something else feels strange on Win7. Not sure exactly what, but it doesn't feel like it fits in with the rest of Win7.
* I didn't test on WinXP, but I suspect the white background would look strange there - since white backgrounds are only used for the headers in wizards on WinXP.
* Didn't test on OSX - do you have any screenshots handy?
* The style of the dialog feels different from the update/incompatible dialogs we have. Which is fine, but if we're going with this style, the other dialogs should really be updated so they're all consistent.

::: toolkit/locales/en-US/chrome/mozapps/extensions/selectAddons.dtd
@@ +1,5 @@
> +<!ENTITY upgrade.title               "&brandShortName; has been upgraded">
> +<!ENTITY upgrade.style               "width: 609px; height: 448px;">
> +
> +<!ENTITY checking.heading            "Checking Your Add-ons">
> +<!ENTITY checking.progress.label     "Checking your add-ons for compatibility with this version of &brandShortName;">

This (and a few of other strings here) should end in a fullstop.

@@ +18,5 @@
> +<!ENTITY action.incompatible.heading "The following add-ons are disabled, but will be enabled as soon as they are compatible:">
> +<!ENTITY action.update.heading       "The following add-ons will be updated:">
> +
> +<!ENTITY update.heading              "Updating Your Add-ons">
> +<!ENTITY update.progress.label       "Downloading and installing updates for your selected add-ons">

Ditto: fullstop.

@@ +20,5 @@
> +
> +<!ENTITY update.heading              "Updating Your Add-ons">
> +<!ENTITY update.progress.label       "Downloading and installing updates for your selected add-ons">
> +
> +<!ENTITY errors.heading              "&brandShortName; could not update some of your add-ons">

Ditto: fullstop.

@@ +21,5 @@
> +<!ENTITY update.heading              "Updating Your Add-ons">
> +<!ENTITY update.progress.label       "Downloading and installing updates for your selected add-ons">
> +
> +<!ENTITY errors.heading              "&brandShortName; could not update some of your add-ons">
> +<!ENTITY errors.description          "Installing updates for some of your add-ons failed. You can try again later by going to the Add-ons Manager">

Ditto: fullstop.

@@ +23,5 @@
> +
> +<!ENTITY errors.heading              "&brandShortName; could not update some of your add-ons">
> +<!ENTITY errors.description          "Installing updates for some of your add-ons failed. You can try again later by going to the Add-ons Manager">
> +
> +<!ENTITY footer.label                "You can always change your add-ons by going to the Add-ons Manager">

Ditto: fullstop.

::: toolkit/locales/en-US/chrome/mozapps/extensions/selectAddons.properties
@@ +5,5 @@
> +action.disabled=will be disabled
> +action.autoupdate=will be updated to be compatible
> +action.incompatible=will be enabled when compatible
> +action.neededupdate=update to make compatible
> +action.unneededupdate=optional update

I think these should be capitalized. Without the capitalization, some of them kinda lead on from the previous column to form a sentence (XXX will be disabled) - but only in English. Others don't even form a sentence in English (XXX optional update).

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +1388,5 @@
>    get autoUpdateDefault() {
>      return AddonManagerInternal.autoUpdateDefault;
> +  },
> +
> +  shouldAutoUpdate: function AM_shouldAutoUpdate(aAddon) {

Originally, this allowed for passing in the value of AddonManager.autoUpdateDefault so it wouldn't be fetched from prefs every time. Its not a big thing for performance, but it might be nice to keep autoUpdateDefault updated via a prefs observer, rather than querying prefs every time (since there are one or two loops that use AddonManager.shouldAutoUpdate).

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +75,5 @@
>  const PREF_XPI_UNPACK                 = "extensions.alwaysUnpack";
>  const PREF_INSTALL_REQUIREBUILTINCERTS = "extensions.install.requireBuiltInCerts";
>  const PREF_INSTALL_DISTRO_ADDONS      = "extensions.installDistroAddons";
>  const PREF_BRANCH_INSTALLED_ADDON     = "extensions.installedDistroAddon.";
> +const PREF_EXTENSIONS_SELECTED        = "extensions.extensionsSelected";

Not sure about the name of this pref - maybe extensions.shownSelectionUI ? (The name of the const is confusing too)

@@ +1704,5 @@
>  
>    /**
>     * Shows the "Compatibility Updates" UI
>     */
>    showMismatchWindow: function XPI_showMismatchWindow() {

Time to rename this method?

::: toolkit/mozapps/extensions/content/selectAddons.css
@@ +49,5 @@
> +  -moz-box-orient: vertical;
> +}
> +
> +.addon:not([optionalupdate]) .addon-action-update,
> +.addon[optionalupdate] .addon-action-message {

You're mixing addon-action and action-addon - would be nice to be consistent.

::: toolkit/mozapps/extensions/content/selectAddons.js
@@ +37,5 @@
> +
> +Components.utils.import("resource://gre/modules/AddonManager.jsm");
> +
> +const Cc = Components.classes;
> +const Ci = Components.interfaces;

Can we use strict mode here? I don't think there's anything here that will break.

@@ +51,5 @@
> +  // do anything else
> +  if (gView != aView)
> +    return;
> +
> +  let view = document.getElementById(gView.nodeID);

Nit: s/view/viewNode/

@@ +349,5 @@
> +    window.close();
> +  }
> +};
> +
> +window.addEventListener("load", function() { showView(gChecking); }, false);

Up to you, but instead of a wrapper function, could also use |showView.bind(null, gChecking)|

::: toolkit/mozapps/extensions/content/selectAddons.xml
@@ +46,5 @@
> +  <binding id="addon">
> +    <content>
> +      <xul:hbox class="select-keep select-cell">
> +        <xul:checkbox anonid="keep"
> +                      oncommand="document.getBindingParent(this).keepChanged()"/>

Style nit: missing semicolon.

@@ +57,5 @@
> +      </xul:hbox>
> +      <xul:hbox class="select-action select-cell">
> +        <xul:label class="addon-action-message" xbl:inherits="xbl:text=action"/>
> +        <xul:checkbox anonid="update" checked="true" class="addon-action-update"
> +                      oncommand="document.getBindingParent(this).updateChanged()"

Style nit: missing semicolon.

@@ +71,5 @@
> +      <field name="_disabled"/>
> +      <field name="_install"/>
> +      <field name="_keep">document.getAnonymousElementByAttribute(this, "anonid", "keep")</field>
> +      <field name="_update">document.getAnonymousElementByAttribute(this, "anonid", "update")</field>
> +      <field name="_strings">document.getElementById("strings")</field>

Style nit: missing semicolons.

::: toolkit/mozapps/extensions/content/selectAddons.xul
@@ +40,5 @@
> +<?xml-stylesheet href="chrome://global/skin/" type="text/css"?> 
> +<?xml-stylesheet href="chrome://mozapps/content/extensions/selectAddons.css" type="text/css"?> 
> +<?xml-stylesheet href="chrome://mozapps/skin/extensions/selectAddons.css" type="text/css"?> 
> +
> +<!DOCTYPE wizard [

Nit: This should be <!DOCTYPE window [
Attachment #543182 - Flags: review?(bmcbride) → review-
(In reply to comment #89)
> Comment on attachment 543182 [details] [diff] [review] [review]
> patch rev 1
> 
> Review of attachment 543182 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> Nothing major to note - just a bunch of small tweaks.
> 
> General notes:
> * Feels like addons should be also ordered by current enable/disable state.
> If there are a bunch of already user disabled addons, they add a lot of
> noise to the list, but little benefit - maybe put them right at the bottom?
> (and/or ask to uninstall them?).
> * It's a little disconcerting when most rows have a blank space in the
> "action" column.
> * The column headers need tweaking on Win7 - the borders look too dark.
> * Something else feels strange on Win7. Not sure exactly what, but it
> doesn't feel like it fits in with the rest of Win7.
> * I didn't test on WinXP, but I suspect the white background would look
> strange there - since white backgrounds are only used for the headers in
> wizards on WinXP.
> * Didn't test on OSX - do you have any screenshots handy?

Going to make screenshots on all platforms and get final ui-review on them, deferring this questions to the UX team to answer.

> * The style of the dialog feels different from the update/incompatible
> dialogs we have. Which is fine, but if we're going with this style, the
> other dialogs should really be updated so they're all consistent.

I think we do want to make the others look like this, don't think it needs to block this patch though.

Addressed the rest of the review comments.
(In reply to comment #90)
> I think we do want to make the others look like this, don't think it needs
> to block this patch though.

Yea, meant that more as a "do we need followups filed?"
Attached image OSX screenshot (obsolete) —
Attached image Linux screenshot (obsolete) —
Attached image Windows screenshot (obsolete) —
Attached are screenshots for the three platforms, would like a final UI check on these
Attachment #546926 - Flags: ui-review?(jboriss)
Attachment #546924 - Flags: ui-review?(jboriss)
Attachment #546925 - Flags: ui-review?(jboriss)
Looking at the column at the right (3rd party/Application/You) it looks strange to not have any label for that column.
See the Linux mockup : is the user supposed to understand what do all these "You" mean ?
Comment on attachment 546926 [details]
Windows screenshot

Oh wait, these are all broken
Attachment #546926 - Attachment is obsolete: true
Attachment #546926 - Flags: ui-review?(jboriss)
Comment on attachment 546926 [details]
Windows screenshot

I guess actually they match the mockups, except the column headings scroll out of view, was this an intentional part of the design?
Attachment #546926 - Flags: ui-review?(jboriss)
Just appeared to me that we're mixing software and humans in the "who installed" column. I wonder if that works in all cultures, CCing some of our Japanese team to get a spot check, as they're hiding either quite frequently, IIRC. Can you guys have a look at the screen shot, and if it's an issue, comment on how you'd like to be able to work around that?
I see some problems also for locales who use an impersonal style: in that case "you" could be localized as "user".

Also: how are those columns acting with longer strings?
Looking at the screenshots: what does the checkbox? Does it disables smth or enables?
(In reply to comment #98)
> Just appeared to me that we're mixing software and humans in the "who
> installed" column. I wonder if that works in all cultures, CCing some of our
> Japanese team to get a spot check, as they're hiding either quite
> frequently, IIRC. Can you guys have a look at the screen shot, and if it's
> an issue, comment on how you'd like to be able to work around that?

I agree with flod.
Using "User" is better for that indicative place.

The word "You" is very emphasized, and repeated "You" is odious for us.
IMO, because of the purpose of this dialog is to show installed by "Third party" or "Application", they will have priorities than "You" on the list.

- source.profile=You
+ source.profile=User
> The word "You" is very emphasized

I mean, "You" is emphasized word in generally.
IE 9 does this.  I installed Skype and it asked me if I wanted to turn on Skype's crapware extension.
Whiteboard: [parity-ie]
(In reply to comment #97)
> Comment on attachment 546926 [details]
> I guess actually they match the mockups, except the column headings scroll
> out of view, was this an intentional part of the design?

The column titles should always be visible, allowing the list to scroll under them but never moving themselves.

(In reply to comment #89)
> Comment on attachment 543182 [details] [diff] [review] [review]
> General notes:
> * Feels like addons should be also ordered by current enable/disable state.
> If there are a bunch of already user disabled addons, they add a lot of
> noise to the list, but little benefit - maybe put them right at the bottom?
> (and/or ask to uninstall them?).

You raise a good point, and I agree that it looks busy and possibly confusing that we'd currently show a bunch of add-ons at the top marked for disabling, but also some further add-ons below it similarly marked without that grouping.  However, I recommend we don't sort the list by enabled/disabled state by default.  To do so would require adding a level of complication by describing add-on's current state as another sorting method.  We'd need an additional column so that users could flick between the current columns but still put the list in the state it originated in.  This screen already is information-dense, and another column I think would only make this worse.

> * It's a little disconcerting when most rows have a blank space in the
> "action" column.

You're right - it's disconcerting.  Here's what I propose: let's show in each column what happens to that add-on if the user presses "Next" at that moment.  So, if an add-on is unchecked, the action is "will be disabled," just like now.  However, if a compatible add-on *is* checked, then the action is "will be enabled."  This not only removes the problem of blank action rows, but allows users to interactively see what happens when they check and uncheck: add-ons go from being enabled to disabled.

> * The column headers need tweaking on Win7 - the borders look too dark.
> * Something else feels strange on Win7. Not sure exactly what, but it
> doesn't feel like it fits in with the rest of Win7.

Blair, could you post a screenshot?

> * I didn't test on WinXP, but I suspect the white background would look
> strange there - since white backgrounds are only used for the headers in
> wizards on WinXP.
> * The style of the dialog feels different from the update/incompatible
> dialogs we have. Which is fine, but if we're going with this style, the
> other dialogs should really be updated so they're all consistent.

My understanding is these were designed to match the style of our upgrade screens on WinXP.  Not so?
Attachment #546926 - Attachment is obsolete: false
(In reply to comment #104)
> (In reply to comment #97)
> > Comment on attachment 546926 [details]
> > I guess actually they match the mockups, except the column headings scroll
> > out of view, was this an intentional part of the design?
> 
> The column titles should always be visible, allowing the list to scroll
> under them but never moving themselves.

Oof, that's going to make this tricky I think...

> > * It's a little disconcerting when most rows have a blank space in the
> > "action" column.
> 
> You're right - it's disconcerting.  Here's what I propose: let's show in
> each column what happens to that add-on if the user presses "Next" at that
> moment.  So, if an add-on is unchecked, the action is "will be disabled,"
> just like now.  However, if a compatible add-on *is* checked, then the
> action is "will be enabled."  This not only removes the problem of blank
> action rows, but allows users to interactively see what happens when they
> check and uncheck: add-ons go from being enabled to disabled.

I think that'll actually have a low effect, I'm guessing the number of add-ons getting enabled by this dialog is going to be low. I think the larger problem is the spaces caused by add-ons where there is no change, either the add-on is staying enabled or staying disabled. Do you want a message like "will remain enabled" there? Seems like this should stay blank to me to emphasise the fact that no change is occuring.

> > * The column headers need tweaking on Win7 - the borders look too dark.
> > * Something else feels strange on Win7. Not sure exactly what, but it
> > doesn't feel like it fits in with the rest of Win7.
> 
> Blair, could you post a screenshot?

Sorry, the screenshot is there, I just accidentally hid it. Should be visible now.

> > * I didn't test on WinXP, but I suspect the white background would look
> > strange there - since white backgrounds are only used for the headers in
> > wizards on WinXP.
> > * The style of the dialog feels different from the update/incompatible
> > dialogs we have. Which is fine, but if we're going with this style, the
> > other dialogs should really be updated so they're all consistent.
> 
> My understanding is these were designed to match the style of our upgrade
> screens on WinXP.  Not so?

It most closely matches the app update UI, though I think it might be a slightly newer iteration of that. If someone could supply a shot of those from XP that'd be awesome.
(In reply to comment #105)
> (In reply to comment #104)
> > The column titles should always be visible, allowing the list to scroll
> > under them but never moving themselves.
> 
> Oof, that's going to make this tricky I think...

I would be delighted to hear that we can mitigate and file a follow up, because I think the feature is high value and I'm keen to get it to users very soon. Is that possible?
(In reply to comment #89)
> ::: toolkit/mozapps/extensions/AddonManager.jsm
> @@ +1388,5 @@
> >    get autoUpdateDefault() {
> >      return AddonManagerInternal.autoUpdateDefault;
> > +  },
> > +
> > +  shouldAutoUpdate: function AM_shouldAutoUpdate(aAddon) {
> 
> Originally, this allowed for passing in the value of
> AddonManager.autoUpdateDefault so it wouldn't be fetched from prefs every
> time. Its not a big thing for performance, but it might be nice to keep
> autoUpdateDefault updated via a prefs observer, rather than querying prefs
> every time (since there are one or two loops that use
> AddonManager.shouldAutoUpdate).

So I tried to do this and then spent a bunch of time debugging the test failures that resulted. Here is the problem:

gDetailsView also observes that pref change to update its check for updates button. It calls the shouldAutoUpdate method in that observer and so would rely on the AddonManagerInternal pref observer firing first. It turns out that the prefs service seems to call observers in the reverse order that they were registered so things go wrong. The only non-hacky (or I guess least-hacky) way I could see around this was to have AddonManagerInternal fire an onPropertyChange event for applyBackgroundUpdates for all affected add-ons in its observer. That's kind of wrong since that property hasn't actually changed, but also complicates testing because setting the pref no longer synchronously updates the UI.

I don't think the slight perf improvement here is worth trying to figure out how to solve that.
> > * The style of the dialog feels different from the update/incompatible
> > dialogs we have. Which is fine, but if we're going with this style, the
> > other dialogs should really be updated so they're all consistent.
> 
> My understanding is these were designed to match the style of our upgrade
> screens on WinXP.  Not so?

The XP wizard looks really cluttered and old.  This isn't fully platform native, but gets the point across to the user (one title instead of three, single color, etc.)  Goal is to remove all existing XP style wizards in the product over time.

>Do you want a message like "will remain enabled" there? Seems like this should >stay blank to me to emphasise the fact that no change is occuring.

My concern is that if the user has to read every line (becuase they can't scan for the interesting part), they will either be frustrated, or move forward without actually comprehending what is about to happen.  So I think we should highlight the changes by positioning them against otherwise whitespace.
Set the window title to be blank on all of these, we are going for one instruction in the content area.
I'm confused as to why Test Add-on 23 and 25 are not listed as "will be disabled"
(In reply to comment #110)
> I'm confused as to why Test Add-on 23 and 25 are not listed as "will be
> disabled"

In the testcase those add-ons were already disabled by the user
Attached image error page
Largely there now, this page could do with a little love though...
Attachment #549966 - Flags: ui-review?(jboriss)
Attached image Linux screenshot
Attachment #546925 - Attachment is obsolete: true
Attachment #550874 - Flags: ui-review?(jboriss)
Attachment #546925 - Flags: ui-review?(jboriss)
Attached image OSX screenshot (obsolete) —
Attachment #546924 - Attachment is obsolete: true
Attachment #550875 - Flags: ui-review?(jboriss)
Attachment #546924 - Flags: ui-review?(jboriss)
Attached image Windows screenshot (obsolete) —
Updated screenshots attached and needing final signoff. You'll note that the larger by default font size on Linux makes some of the lines wrap, I could shrink the fontsize down if absolutely necessary I guess but it seems ok as-is
Attachment #546926 - Attachment is obsolete: true
Attachment #550877 - Flags: ui-review?(jboriss)
Attachment #546926 - Flags: ui-review?(jboriss)
Comment on attachment 550877 [details]
Windows screenshot

I think I figured out what feels out of place on Win7 - Windows doesn't do centered text for headings. And specifically in Win7, such headings in dialogs are a smaller fontsize, left aligned, and dark blue. eg: http://grab.by/aFPM and http://grab.by/aFPT
I don't understand what the difference between "3drd party" and "Application"
Is these distinction relevant BTW ?
From my user point of view there is "Me" (i've decided to have this) and "Others" (not my choice)
(In reply to antistress from comment #117)
> I don't understand what the difference between "3drd party" and "Application"
> Is these distinction relevant BTW ?
> From my user point of view there is "Me" (i've decided to have this) and
> "Others" (not my choice)

"Application" means add-ons shipped with the application, maybe it should say "Firefox"
Might be worth noting that we commonly use add-ons shipped with the download for partner builds. I don't have a current example on my head, but when you used to download the "firefox ebay edition", you'd get the "ebay companion" together with your download. For those, it makes sense to show them as distinct from 3rd party and you, as they're part of what you downloaded, but only somewhat explicitly installed by you.
(In reply to comment #105)
> > You're right - it's disconcerting.  Here's what I propose: let's show in
> > each column what happens to that add-on if the user presses "Next" at that
> > moment.  So, if an add-on is unchecked, the action is "will be disabled,"
> > just like now.  However, if a compatible add-on *is* checked, then the
> > action is "will be enabled."  This not only removes the problem of blank
> > action rows, but allows users to interactively see what happens when they
> > check and uncheck: add-ons go from being enabled to disabled.
> 
> I think that'll actually have a low effect, I'm guessing the number of
> add-ons getting enabled by this dialog is going to be low. I think the
> larger problem is the spaces caused by add-ons where there is no change,
> either the add-on is staying enabled or staying disabled. Do you want a
> message like "will remain enabled" there? Seems like this should stay blank
> to me to emphasise the fact that no change is occuring.

Actually, this is the case I was referring to - I was suggesting we say "will be enabled" even for add-ons which were previously enabled.  The phrase "Will remain enabled" is even better, since it gives the full story, but either of these is better than blank.  A blank entry will tell a user nothing rather than that no change is happening.  By giving information we allow users to parse the screen by ignoring the explanatory text and just looking at the columns.

> > > * The column headers need tweaking on Win7 - the borders look too dark.
> > > * Something else feels strange on Win7. Not sure exactly what, but it
> > > doesn't feel like it fits in with the rest of Win7.

You're right, those are far too dark.  Also, the dividing lines below the column headers don't extend across the entire window horizontally, making them look fragmented and fugly.  Could we take the opacity of the  lines down to 34%, making them appear #e0e0e0?  I attached a mockup of this.

(In reply to comment #106)
> (In reply to comment #105)
> > (In reply to comment #104)
> I would be delighted to hear that we can mitigate and file a follow up,
> because I think the feature is high value and I'm keen to get it to users
> very soon. Is that possible?

I'll do that.  It's not worth blocking this on - I imagine most users won't have enough add-ons to show a scrollbar.
If I may ask, can we use more win7-like styled window?
(In reply to Jennifer Boriss [:Boriss] (Firefox UX) from comment #120)
> (In reply to comment #106)
> > (In reply to comment #105)
> > > (In reply to comment #104)
> > I would be delighted to hear that we can mitigate and file a follow up,
> > because I think the feature is high value and I'm keen to get it to users
> > very soon. Is that possible?
> 
> I'll do that.  It's not worth blocking this on - I imagine most users won't
> have enough add-ons to show a scrollbar.

Do we have telemetry or test pilot data to back this statement up?
(In reply to Josh Matthews [:jdm] from comment #122)
> (In reply to Jennifer Boriss [:Boriss] (Firefox UX) from comment #120)
> > (In reply to comment #106)
> > > (In reply to comment #105)
> > > > (In reply to comment #104)
> > > I would be delighted to hear that we can mitigate and file a follow up,
> > > because I think the feature is high value and I'm keen to get it to users
> > > very soon. Is that possible?
> > 
> > I'll do that.  It's not worth blocking this on - I imagine most users won't
> > have enough add-ons to show a scrollbar.
> 
> Do we have telemetry or test pilot data to back this statement up?

I think we have a fair amount of soft data that says this isn't the case, hence why I spent the time to come up with a solution to it already. The latest screenshots show this
Comment on attachment 551514 [details]
Windows 7 screen with lighter lines, lines extended to full horizontal width

BTW The design is not compatible with Win tablets.
(In reply to Eugene Savitsky from comment #124)
> Comment on attachment 551514 [details]
> Windows 7 screen with lighter lines, lines extended to full horizontal width
> 
> BTW The design is not compatible with Win tablets.

In what way?
Can you easily point to a small check box with a finger?
(In reply to Eugene Savitsky from comment #126)
> Can you easily point to a small check box with a finger?

That seems like a problem that would affect all of the preferences UI and lots of other places in Firefox already, or is there something special about this case?
Sure, you may not have to redesign and refactor it once again in the very near future.
(In reply to Josh Matthews [:jdm] from comment #122)
> Do we have telemetry or test pilot data to back this statement up?

Yes, we do. http://66.43.220.232/james/extensions-tool.html is a pretty nice visualization of that.
Attached image Windows screenshot
Updated the windows styles from your suggestions. Waiting for ui-r on the styles of the different platforms and a suggestion for what to do with the error page
Attachment #550877 - Attachment is obsolete: true
Attachment #551514 - Attachment is obsolete: true
Attachment #551605 - Flags: ui-review?(jboriss)
Attachment #550877 - Flags: ui-review?(jboriss)
Attachment #551605 - Attachment is patch: false
Attachment #551605 - Attachment mime type: text/plain → image/png
Comment on attachment 551514 [details]
Windows 7 screen with lighter lines, lines extended to full horizontal width

Looks good, but lines should be #D6E5F5 for maximum windows nativey-ness
Attachment #551514 - Flags: feedback+
> BTW The design is not compatible with Win tablets.

Windows 7 is not physically compatible with Windows 7 tablets.
Comment on attachment 550875 [details]
OSX screenshot

remove the greytext and italics from the message at the bottom.
Attachment #550875 - Flags: feedback-
>"Application" means add-ons shipped with the application,
>maybe it should say "Firefox"

That seems fine, application does create the question of "which application?"
Attached image OSX screenshot
Updated the OSX screenshot per faaborg's comment. I've also switched to the colour he suggested for windows, not going to bother with a new screenshot for that though.
Attachment #550875 - Attachment is obsolete: true
Attachment #551678 - Flags: ui-review?(jboriss)
Attachment #550875 - Flags: ui-review?(jboriss)
>The phrase "Will remain enabled" is even better, since it gives the full story, but either of these is better
>than blank.  A blank entry will tell a user nothing rather than that no change is happening.  By giving
>information we allow users to parse the screen by ignoring the explanatory text and just looking at the
>columns.

I disagree, using whitespace to emphasize the things that actually are about to happen (as opposed to listing the status of every single item) makes it harder for users to parse what is about to happen.  of course we are kind of hoping that users will give up and click next (since it makes Firefox considerably faster and more stable with all of the forced in third party extensions disabled), but overall I think a column that can hold 7 (?) different values is going to significantly confuse users as they read each entry.
(In reply to Alex Faaborg [:faaborg] (Firefox UX) from comment #131)
> Comment on attachment 551514 [details]
> Windows 7 screen with lighter lines, lines extended to full horizontal width
> 
> Looks good, but lines should be #D6E5F5 for maximum windows nativey-ness

Sounds good.

(In reply to Alex Faaborg [:faaborg] (Firefox UX) from comment #136)
> >The phrase "Will remain enabled" is even better, since it gives the full story, but either of these is better
> >than blank.  A blank entry will tell a user nothing rather than that no change is happening.  By giving
> >information we allow users to parse the screen by ignoring the explanatory text and just looking at the
> >columns.
> 
> I disagree, using whitespace to emphasize the things that actually are about
> to happen (as opposed to listing the status of every single item) makes it
> harder for users to parse what is about to happen.  of course we are kind of
> hoping that users will give up and click next (since it makes Firefox
> considerably faster and more stable with all of the forced in third party
> extensions disabled), but overall I think a column that can hold 7 (?)
> different values is going to significantly confuse users as they read each
> entry.

Fine, let's do that.

Dave, the only problem I see is that the text in attachment 549966 [details] stretches too far.  Can we make that a bit smaller?
Reminder, we'd still like to get answers on how this should look for locales that don't take "you" well, see comment 98 through comment 102
(In reply to Axel Hecht [:Pike] from comment #138)
> Reminder, we'd still like to get answers on how this should look for locales
> that don't take "you" well, see comment 98 through comment 102

So it sounds like a viable option would be to have those locales use the equivalent of "User" in those cases, how does this sound Boriss?

If not what is the best way to get answers on this Axel, a post to the l10n newsgroup?
(In reply to Dave Townsend (:Mossop) from comment #139)
> (In reply to Axel Hecht [:Pike] from comment #138)
> > Reminder, we'd still like to get answers on how this should look for locales
> > that don't take "you" well, see comment 98 through comment 102
> 
> So it sounds like a viable option would be to have those locales use the
> equivalent of "User" in those cases, how does this sound Boriss?
> 
> If not what is the best way to get answers on this Axel, a post to the l10n
> newsgroup?

Thanks for the heads-up, Axel.  I'd really like to keep the word "you" as the English string, but perhaps some further explanation is needed for languages using an impersonal style, like Japanese.  Masahiko Imanaka, would it help if we describe third-party add-on installation constituting an act that happened without action from the user, while other add-on installation happened as a result of action from the user?


Perhaps someone with more localization knowledge can chime in with exactly how this is a problem for specific languages.  In short, if the concept of "you" isn't helpful, the instructions should make the distinction between an act the user has caused and one that happened without their action.
Attachment #550874 - Flags: ui-review?(jboriss) → ui-review+
Attachment #551605 - Flags: ui-review?(jboriss) → ui-review+
Attachment #551678 - Flags: ui-review?(jboriss) → ui-review+
So far, it seems that the languages we have commenting here in the bug can work with "user" instead of "You". For that, a localization note in the string would be good. Also one that describes the difference between application and third party.

My concern was probably something like:

if a locale works around "you" by saying something as wordy as "utilisator ordinatorum" (not latin, forbid), the current UX might strike a different balance with other ideas you might have.

I wouldn't have any alternatives, but if your reasoning for the design was "let's put YOU here, that's short, so it's OK", then that might not generally apply.


Which brings me a tad further, Dave, can you create a mozmill test that can open up this view with a bunch of test addons so that we can do screenshots and do a health check across all our builds in the aurora phase? That might be the most thorough way to get some distributed data, and guide us if we want to revisit the design?
(In reply to Axel Hecht [:Pike] from comment #141)
> Which brings me a tad further, Dave, can you create a mozmill test that can
> open up this view with a bunch of test addons so that we can do screenshots
> and do a health check across all our builds in the aurora phase? That might

Mozmill will not work at this phase of the process. It's an add-on and will not be active yet. If I'm wrong please correct me Dave. But it looks like there is no Mozmill automation involved here.
(In reply to Axel Hecht [:Pike] from comment #141)
> Which brings me a tad further, Dave, can you create a mozmill test that can
> open up this view with a bunch of test addons so that we can do screenshots
> and do a health check across all our builds in the aurora phase? That might
> be the most thorough way to get some distributed data, and guide us if we
> want to revisit the design?

The automated test here already opens up the view with some test add-ons. Are the requirements here that it be mozmill based, could it f.e. just be an extension?
Attached patch patch rev 2Splinter Review
This is the current state of the patch, I think it is worth getting a review again now. Hopefully the only questions remaining to be answered are l10n ones. There is some suckiness here to keep the column headings in view. Basically the only way I could find to do it was to use fixed column sizes for all but the last column. Because many of those columns contain text that may be of varying lengths I have included the widths of those columns in the locale so both the window size and the column sizes can be controlled by the locale. If all else fails the text within the table will wrap if it gets too long which while not ideal does mean that bad column sizing doesn't make the dialog unusable.

Axel, I'd like your feedback on that and the existing localisation notes.
Attachment #543182 - Attachment is obsolete: true
Attachment #551975 - Flags: review?(bmcbride)
Attachment #551975 - Flags: feedback?(l10n)
Dave, could we please get a tryserver build based on your latest patch? We would like to start testing this feature as soon as possible. Thanks.
Status: NEW → ASSIGNED
Comment on attachment 551975 [details] [diff] [review]
patch rev 2

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

::: toolkit/locales/en-US/chrome/mozapps/extensions/selectAddons.dtd
@@ +7,5 @@
> +<!ENTITY select.description          "Make &brandShortName; even faster by disabling add-ons you no longer use. Add-ons already installed by third parties will be disabled automatically unless you select them below.">
> +<!ENTITY select.keep                 "Keep">
> +<!-- LOCALIZATION NOTE (select.keep.style): Should be a width wide enough for
> +     the string in select.keep above. -->
> +<!ENTITY select.keep.style           "width: 45px;">

Could this (and other similar strings) just be "45px"? It feels wrong to make localisers deal with CSS syntax.

@@ +31,5 @@
> +<!ENTITY update.heading              "Updating Your Add-ons">
> +<!ENTITY update.progress.label       "Downloading and installing updates for your selected add-ons.">
> +
> +<!ENTITY errors.heading              "&brandShortName; could not update some of your add-ons.">
> +<!ENTITY errors.description          "Installing updates for some of your add-ons failed. You can try again later by going to the Add-ons Manager.">

This isn't entirely accurate - in the normal case, the Addons Manager will try to update them periodically without user action. This string makes it sound like they need to manually update them.

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +481,5 @@
>      this.providers.forEach(function(provider) {
>        callProvider(provider, "shutdown");
>      });
>  
> +    Services.prefs.removeObserver(PREF_EM_AUTOUPDATE_DEFAULT, this);

Why is this here?

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +1707,5 @@
>    /**
>     * Shows the "Compatibility Updates" UI
>     */
> +  showUpgradeUI: function XPI_showUpgradeUI() {
> +    if (!Prefs.getBoolPref(PREF_SHOW_SELECTION_UI, false)) {

This pref is used backwards to what the name suggests. As in, the code is doing: if (!show) showUI()

::: toolkit/mozapps/extensions/content/selectAddons.js
@@ +145,5 @@
> +    gAddons[aAddon.id].install = aInstall;
> +  },
> +
> +  onNoUpdateAvailable: function(aAddon) {
> +  },

Nit: Keep the } on the same line, or add a comment saying "do nothing". Saves wondering if the method is intentionally empty later on.

@@ +169,5 @@
> +
> +    let rows = document.getElementById("select-rows");
> +    let lastAddon = null;
> +    addons.forEach(function(aEntry) {
> +      if (lastAddon && orderForScope(aEntry.addon.scope) != orderForScope(lastAddon.scope)) {

Nit: Overly long line.

@@ +198,5 @@
> +    this.updateButtons();
> +  },
> +
> +  updateButtons: function() {
> +    for (let row = document.getElementById("select-rows").firstChild; row; row = row.nextSibling) {

Nit: Overly long line.

@@ +234,5 @@
> +        box.removeChild(box.lastChild);
> +      box = box.nextSibling;
> +    }
> +
> +    for (let row = document.getElementById("select-rows").firstChild; row; row = row.nextSibling) {

Nit: Overly long line.

@@ +240,5 @@
> +        continue;
> +
> +      let list = document.getElementById(row.action + "-list");
> +      if (!list)
> +        continue;

Seems this should only be hit of row.action is null, in which case you should just check for that directly.

@@ +266,5 @@
> +    showView(gUpdate);
> +  },
> +
> +  done: function() {
> +    for (let row = document.getElementById("select-rows").firstChild; row; row = row.nextSibling) {

Nit: Overly long line.

@@ +288,5 @@
> +    showButtons(true, false, false, false);
> +
> +    this._progress = document.getElementById("update-progress");
> +
> +    for (let row = document.getElementById("select-rows").firstChild; row; row = row.nextSibling) {

Nit: Overly long line.
Attachment #551975 - Flags: review?(bmcbride) → review+
Comment on attachment 551975 [details] [diff] [review]
patch rev 2

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

Threw a dice on feedback+ or -. Went for the latter as I'd like to see some changes made.

::: toolkit/locales/en-US/chrome/mozapps/extensions/selectAddons.dtd
@@ +3,5 @@
> +<!ENTITY checking.heading            "Checking Your Add-ons">
> +<!ENTITY checking.progress.label     "Checking your add-ons for compatibility with this version of &brandShortName;.">
> +
> +<!ENTITY select.heading              "Select Your Add-ons">
> +<!ENTITY select.description          "Make &brandShortName; even faster by disabling add-ons you no longer use. Add-ons already installed by third parties will be disabled automatically unless you select them below.">

Want to add a comment that "third party" here should match the translation of source.other in selectAddons.properties?

@@ +7,5 @@
> +<!ENTITY select.description          "Make &brandShortName; even faster by disabling add-ons you no longer use. Add-ons already installed by third parties will be disabled automatically unless you select them below.">
> +<!ENTITY select.keep                 "Keep">
> +<!-- LOCALIZATION NOTE (select.keep.style): Should be a width wide enough for
> +     the string in select.keep above. -->
> +<!ENTITY select.keep.style           "width: 45px;">

Also, should this be px or ex? I wonder how this deal with DPIs and large font sizes.

@@ +11,5 @@
> +<!ENTITY select.keep.style           "width: 45px;">
> +<!ENTITY select.action               "Action">
> +<!-- LOCALIZATION NOTE (select.action.style): Should be a width wide enough for
> +     the action strings in selectAddons.properties. -->
> +<!ENTITY select.action.style         "width: 200px;">

This also needs room for brandShortName, right?

@@ +17,5 @@
> +<!ENTITY select.name                 "Name">
> +<!-- LOCALIZATION NOTE (select.name.style): Should be a width small enough so
> +     the source column still has enough room for the source strings in
> +     selectAddons.properties. -->
> +<!ENTITY select.name.style           "width: 300px;">

What happens if that's not the case? Can we use CSS math here now that we have it?

@@ +20,5 @@
> +     selectAddons.properties. -->
> +<!ENTITY select.name.style           "width: 300px;">
> +
> +<!ENTITY confirm.heading             "Select Your Add-ons">
> +<!ENTITY confirm.description         "Make &brandShortName; even faster by disabling add-ons you no longer use. Add-ons already installed by third parties will be disabled automatically unless you select them below.">

Also cross reference the "third party" here? This is less of an issue for folks with plain editors, but for the guys that use "smart editors", the strings are going to just show up in a single text field.

::: toolkit/locales/en-US/chrome/mozapps/extensions/selectAddons.properties
@@ +1,2 @@
> +#LOCALIZATION NOTE (source.profile) add-ons installed by the user
> +source.profile=You

Can you add that localizers can either use "You" or "User" as original string, depending on which better maps the tone in that the localization addresses the user?
Attachment #551975 - Flags: feedback?(l10n) → feedback-
(In reply to Dave Townsend (:Mossop) from comment #143)
> (In reply to Axel Hecht [:Pike] from comment #141)
> > Which brings me a tad further, Dave, can you create a mozmill test that can
> > open up this view with a bunch of test addons so that we can do screenshots
> > and do a health check across all our builds in the aurora phase? That might
> > be the most thorough way to get some distributed data, and guide us if we
> > want to revisit the design?
> 
> The automated test here already opens up the view with some test add-ons.
> Are the requirements here that it be mozmill based, could it f.e. just be an
> extension?

I was suggesting mozmill, because we have infra to take screenshots in that as part of a test. That comes in handy if you're doing that 90 times, possibly by platform. Do we have something that fits this?
(In reply to Axel Hecht [:Pike] from comment #147)
> > +<!ENTITY select.keep.style           "width: 45px;">
> 
> Also, should this be px or ex? I wonder how this deal with DPIs and large
> font sizes.

Widths in UI should ideally always be in ch units (and heights in em), as those are units that change correctly with (system) font type/size prefs.
(In reply to Blair McBride (:Unfocused) from comment #146)
> > +<!ENTITY select.description          "Make &brandShortName; even faster by disabling add-ons you no longer use. Add-ons already installed by third parties will be disabled automatically unless you select them below.">
> > +<!ENTITY select.keep                 "Keep">
> > +<!-- LOCALIZATION NOTE (select.keep.style): Should be a width wide enough for
> > +     the string in select.keep above. -->
> > +<!ENTITY select.keep.style           "width: 45px;">
> 
> Could this (and other similar strings) just be "45px"? It feels wrong to
> make localisers deal with CSS syntax.

Axel, what is your take on this? I've generally seen window styles done this way so it made sense for these styles to be the same.
(In reply to Blair McBride (:Unfocused) from comment #146)
> > +<!ENTITY update.heading              "Updating Your Add-ons">
> > +<!ENTITY update.progress.label       "Downloading and installing updates for your selected add-ons.">
> > +
> > +<!ENTITY errors.heading              "&brandShortName; could not update some of your add-ons.">
> > +<!ENTITY errors.description          "Installing updates for some of your add-ons failed. You can try again later by going to the Add-ons Manager.">
> 
> This isn't entirely accurate - in the normal case, the Addons Manager will
> try to update them periodically without user action. This string makes it
> sound like they need to manually update them.

Boriss/faaborg?
(In reply to Blair McBride (:Unfocused) from comment #146)
> @@ +481,5 @@
> >      this.providers.forEach(function(provider) {
> >        callProvider(provider, "shutdown");
> >      });
> >  
> > +    Services.prefs.removeObserver(PREF_EM_AUTOUPDATE_DEFAULT, this);
> 
> Why is this here?

Accidentally included from a different patch I think, removed.

> ::: toolkit/mozapps/extensions/XPIProvider.jsm
> @@ +1707,5 @@
> >    /**
> >     * Shows the "Compatibility Updates" UI
> >     */
> > +  showUpgradeUI: function XPI_showUpgradeUI() {
> > +    if (!Prefs.getBoolPref(PREF_SHOW_SELECTION_UI, false)) {
> 
> This pref is used backwards to what the name suggests. As in, the code is
> doing: if (!show) showUI()

Changed s/show/shown/ throughout

> ::: toolkit/mozapps/extensions/content/selectAddons.js
> @@ +145,5 @@
> > +    gAddons[aAddon.id].install = aInstall;
> > +  },
> > +
> > +  onNoUpdateAvailable: function(aAddon) {
> > +  },
> 
> Nit: Keep the } on the same line, or add a comment saying "do nothing".
> Saves wondering if the method is intentionally empty later on.

Just removed it, it's unnecessary now, I also slightly simplified the onUpdateAvailable function.
(In reply to Axel Hecht [:Pike] from comment #147)
> @@ +17,5 @@
> > +<!ENTITY select.name                 "Name">
> > +<!-- LOCALIZATION NOTE (select.name.style): Should be a width small enough so
> > +     the source column still has enough room for the source strings in
> > +     selectAddons.properties. -->
> > +<!ENTITY select.name.style           "width: 300px;">
> 
> What happens if that's not the case? Can we use CSS math here now that we
> have it?

In the worst case the text in that column would linewrap, I'll check what happens if it is too small for that.
(In reply to Axel Hecht [:Pike] from comment #141)
> My concern was probably something like:
> 
> if a locale works around "you" by saying something as wordy as "utilisator
> ordinatorum" (not latin, forbid), the current UX might strike a different
> balance with other ideas you might have.
> 
> I wouldn't have any alternatives, but if your reasoning for the design was
> "let's put YOU here, that's short, so it's OK", then that might not
> generally apply.

It sounds like there's not necessarily an l10n problem here, then, as long as we have a localization note explaining.  The "you" in English was meant to provide further definition between action's the user has taken directly and third-party actions.  As long as the separation between user-initiated and system-initiated is there, we're fine.

(In reply to Dave Townsend (:Mossop) from comment #144)
... Hopefully the only questions .... Because many of those columns contain text that
> may be of varying lengths I have included the widths of those columns in the
> locale so both the window size and the column sizes can be controlled by the
> locale. If all else fails the text within the table will wrap if it gets too
> long which while not ideal does mean that bad column sizing doesn't make the ....

Sounds good.  Users only experience this dialog once, after all, so usable and faster trumps perfect and next release.

(In reply to Dave Townsend (:Mossop) from comment #151)
> (In reply to Blair McBride (:Unfocused) from comment #146)
> > > +<!ENTITY update.heading              "Updating Your Add-ons">
> > > +<!ENTITY update.progress.label       "Downloading and installing updates for your selected add-ons.">
> > > +
> > > +<!ENTITY errors.heading              "&brandShortName; could not update some of your add-ons.">
> > > +<!ENTITY errors.description          "Installing updates for some of your add-ons failed. You can try again later by going to the Add-ons Manager.">
> > 
> > This isn't entirely accurate - in the normal case, the Addons Manager will
> > try to update them periodically without user action. This string makes it
> > sound like they need to manually update them.
> 
> Boriss/faaborg?

Good catch - this indeed implies that updates are manual.  If we have this message, it should really say "Installing updates for some of your add-ons failed. Firefox will automatically try to update them again later."

Though really - could we get away with not displaying this error message at all?  It's hard to imagine the user will care about updates they were previously not using, previously probably didn't know about, and require no action.  Is it the case that some updates, if they fail, will lead to the add-on being incompatible and subsequently entirely disabled on upgrade?
(In reply to Axel Hecht [:Pike] from comment #148)
> (In reply to Dave Townsend (:Mossop) from comment #143)
> > (In reply to Axel Hecht [:Pike] from comment #141)
> > > Which brings me a tad further, Dave, can you create a mozmill test that can
> > > open up this view with a bunch of test addons so that we can do screenshots
> > > and do a health check across all our builds in the aurora phase? That might
> > > be the most thorough way to get some distributed data, and guide us if we
> > > want to revisit the design?
> > 
> > The automated test here already opens up the view with some test add-ons.
> > Are the requirements here that it be mozmill based, could it f.e. just be an
> > extension?
> 
> I was suggesting mozmill, because we have infra to take screenshots in that
> as part of a test. That comes in handy if you're doing that 90 times,
> possibly by platform. Do we have something that fits this?

Ok that is a good reason. I should be able to make something mozmill to do this. It seems like we shouldn't need to block on this for landing on trunk though, I'll come back to it after that.
> (In reply to Dave Townsend (:Mossop) from comment #151)
> > (In reply to Blair McBride (:Unfocused) from comment #146)
> > > > +<!ENTITY update.heading              "Updating Your Add-ons">
> > > > +<!ENTITY update.progress.label       "Downloading and installing updates for your selected add-ons.">
> > > > +
> > > > +<!ENTITY errors.heading              "&brandShortName; could not update some of your add-ons.">
> > > > +<!ENTITY errors.description          "Installing updates for some of your add-ons failed. You can try again later by going to the Add-ons Manager.">
> > > 
> > > This isn't entirely accurate - in the normal case, the Addons Manager will
> > > try to update them periodically without user action. This string makes it
> > > sound like they need to manually update them.
> > 
> > Boriss/faaborg?
> 
> Good catch - this indeed implies that updates are manual.  If we have this
> message, it should really say "Installing updates for some of your add-ons
> failed. Firefox will automatically try to update them again later."
> 
> Though really - could we get away with not displaying this error message at
> all?  It's hard to imagine the user will care about updates they were
> previously not using, previously probably didn't know about, and require no
> action.  Is it the case that some updates, if they fail, will lead to the
> add-on being incompatible and subsequently entirely disabled on upgrade?

Exactly this. Some add-ons will become disabled by the application update and the update is needed to make them work again. I'll use your new string.
(In reply to Dave Townsend (:Mossop) from comment #150)
> (In reply to Blair McBride (:Unfocused) from comment #146)
> > Could this (and other similar strings) just be "45px"? It feels wrong to
> > make localisers deal with CSS syntax.
> 
> Axel, what is your take on this? I've generally seen window styles done this
> way so it made sense for these styles to be the same.

I don't think this matters much. I've seen folks transliterate numbers and px. Sticking to a real css spec makes it easier to write tests for this at some point, so I'd rather opt for keeping it in full. Also, some folks do max-width vs min-width vs width.
Error page looks good aside from the string change and one more tweak: it needs a line break on the main string so we get some larger margins.
Attached patch patch rev 3Splinter Review
Hopefully final patch
Attachment #552228 - Flags: review+
Attachment #552228 - Flags: feedback?(l10n)
(In reply to Henrik Skupin (:whimboo) from comment #145)
> Dave, could we please get a tryserver build based on your latest patch? We
> would like to start testing this feature as soon as possible. Thanks.

Builds will appear here: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dtownsend@mozilla.com-0f0075dcdb9a/
the thing is I want my addons to be still enabled on upgrade. specially adblock plus.
Comment on attachment 552228 [details] [diff] [review]
patch rev 3

looks good to me, too.
Attachment #552228 - Flags: feedback?(l10n) → feedback+
(In reply to remixedcat from comment #161)
> the thing is I want my addons to be still enabled on upgrade. specially
> adblock plus.

Then don't choose to disable those add-ons when prompted
(In reply to remixedcat from comment #161)
> the thing is I want my addons to be still enabled on upgrade. specially
> adblock plus.

If you ignore this screen and press next, your add-ons like adblock plus will still be enabled.
What happens if the user presses escape (on Mac) or closes the dialog (on Win).

Are we going to allow that?
Dave, to be consistent with the Add-ons Manager we should also keep the order of actions to perform on the second wizard page. Means we should list those add-ons first which will be updated and not the disabled ones. See the screenshot.
(In reply to Henrik Skupin (:whimboo) from comment #166)
> Created attachment 552687 [details]
> Screenshot (disabled/be updated)
> 
> Dave, to be consistent with the Add-ons Manager we should also keep the
> order of actions to perform on the second wizard page. Means we should list
> those add-ons first which will be updated and not the disabled ones. See the
> screenshot.

I disagree. I think the most important case here is that some things will no longer be available to the user so I think that is what we should list first.
>I think the most important case here is that some things will no longer be >available to the user so I think that is what we should list first.

yeah, comprehension of the change is considerably more important than a consistent ordering with the add-ons manager.
Landed: http://hg.mozilla.org/mozilla-central/rev/6d6e8fecaf37

There is a bunch of automated testing here that opens the UI and checks it works as expected, no testing that the UI shows up correctly on upgrade though so that would be useful in a manual test.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Keywords: uiwanted
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
Backed out due to some bustage caused by theme changes on OSX: http://hg.mozilla.org/mozilla-central/rev/cec797d85529
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch theme fixSplinter Review
Theme fix for bug 667480
Attachment #552797 - Flags: review?(robert.bugzilla)
Attachment #552797 - Flags: review?(robert.bugzilla) → review+
Re-landed with the fix: http://hg.mozilla.org/mozilla-central/rev/73c4423aafee
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Comment on attachment 552228 [details] [diff] [review]
patch rev 3

>+++ b/toolkit/themes/winstripe/mozapps/extensions/selectAddons.css

>+#footer {
>+  padding: 15px 12px;
>+  background-color: #f1f5fb;
>+  box-shadow: 0px 1px 2px rgb(204,214,234) inset;
>+}
>+
>+.progress-label,
>+#footer-label {
>+  font-style: italic;
>+  color: GrayText;
>+}

This is entirely broken. The footer styling won't fit non-aero themes such as any theme on Windows XP and the label is guaranteed to be invisible with dark (e.g. high-contrast) themes, where GrayText isn't gray. Please file a bug on fixing this?
Attachment #552228 - Flags: review-
Component: General → Add-ons Manager
Product: Firefox → Toolkit
QA Contact: general → add-ons.manager
Target Milestone: Firefox 8 → mozilla8
Depends on: 678761
(In reply to Dão Gottwald [:dao] from comment #173)
> Comment on attachment 552228 [details] [diff] [review]
> patch rev 3
> 
> >+++ b/toolkit/themes/winstripe/mozapps/extensions/selectAddons.css
> 
> >+#footer {
> >+  padding: 15px 12px;
> >+  background-color: #f1f5fb;
> >+  box-shadow: 0px 1px 2px rgb(204,214,234) inset;
> >+}
> >+
> >+.progress-label,
> >+#footer-label {
> >+  font-style: italic;
> >+  color: GrayText;
> >+}
> 
> This is entirely broken. The footer styling won't fit non-aero themes such
> as any theme on Windows XP and the label is guaranteed to be invisible with
> dark (e.g. high-contrast) themes, where GrayText isn't gray. Please file a
> bug on fixing this?

Filed bug 678761 and requested tracking-firefox8 on it
The description text on the confirmation screen is copied verbatim from the selection screen.

"Make Firefox even faster by disabling add-ons you no longer use. Add-ons already installed by third parties will be disabled automatically unless you select them below."

But it's not possible to make any selections on the confirmation screen. Should it not rather say something like "Review the following changes that we will make when you click Done. If you wish to make any changes, click Back."
Depends on: 678828
Depends on: 679157
Depends on: 679795
Depends on: 679802
I'm attaching a screenshot of the problem we are having in the es-ES localization, I think that the size of the content is not kept with the same width than the column title.
(In reply to Guillermo López (:willyaranda) from comment #176)
> Created attachment 553920 [details]
> es-ES problem with first column
> 
> I'm attaching a screenshot of the problem we are having in the es-ES
> localization, I think that the size of the content is not kept with the same
> width than the column title.

Increasing select.keep.style isn't helping?
Exactly the same problem with Italian (it). Changing select.keep.style fixed it.

@dave
As I wrote in mozilla.dev.l10n, we need a testing strategy to check all possible situations. For example, I have no idea when the confirmation screen is displayed, so I have the same doubts expressed in comment 175
Depends on: 680117
Depends on: 680114
Depends on: 680113
Depends on: 679588
No longer depends on: 679795
(comment #177 thanks! I didn't see that variable, it works now)
idk which screen but one of them has a cut next button , checked in nightly 9. Will try to post screenshot
(In reply to bogas04 from comment #180)
> idk which screen but one of them has a cut next button , checked in nightly
> 9. Will try to post screenshot

It's probably bug 680117 (in the meantime you can increase width in upgrade.style).
Depends on: 680643
Depends on: 680873
Depends on: 680874
Attached file Mozmill automation v1 (obsolete) —
Took me longer than I had hoped to get this and I'm not sure exactly how to make this most useful to localisers, but if you download this and run it using the mozmill extension then it will simulate a number of add-ons and open the UI so you can see what it looks like and tinker with it.

Let me know what else would be useful here.
Attachment #556092 - Attachment mime type: application/x-javascript → text/plain
(In reply to Dave Townsend (:Mossop) from comment #182)
> Took me longer than I had hoped to get this and I'm not sure exactly how to
> make this most useful to localisers, but if you download this and run it
> using the mozmill extension then it will simulate a number of add-ons and
> open the UI so you can see what it looks like and tinker with it.

Thanks Dave. That's a big help! I have now modified the test and made a patch out of it so it can handle the modal dialog and also use the screenshot feature from the mozmill-tests repository. That way I can now run this test against all localized builds. I will upload screenshots and report later the location.
Attachment #556092 - Attachment is obsolete: true
Axel, what is the progress on getting this dialog localized? I did a run across all locales for the latest Aurora build but for most of those builds nothing has been translated yet. So the screenshots aren't helpful at the moment. When can we expect localized content?
(In reply to Henrik Skupin (:whimboo) from comment #184)
> Axel, what is the progress on getting this dialog localized? 

Non sure if we're talking about the same thing, but Italian had these strings localized at least two weeks ago and I could make fixes using nightly builds (e.g.  680117).
We are talking about the same bug, yes. I have now uploaded all the screenshots to my people account: http://people.mozilla.com/~hskupin/screenshots/select_addons/

I really would like to know from the l10n drivers when we can expect everything to be localized. QA can't sign off this feature without knowing that other locales also don't break the ui.

Thanks Flod for being so fast and having the translation already done! That was kinda helpful.
(In reply to Henrik Skupin (:whimboo) from comment #186)
> We are talking about the same bug, yes. I have now uploaded all the
> screenshots to my people account:
> http://people.mozilla.com/~hskupin/screenshots/select_addons/
> 
> I really would like to know from the l10n drivers when we can expect
> everything to be localized. QA can't sign off this feature without knowing
> that other locales also don't break the ui.
> 
> Thanks Flod for being so fast and having the translation already done! That
> was kinda helpful.

why is there no screenshot of 'nl', we have everything translated.
(In reply to Tim Maks van den Broek from comment #187)
> why is there no screenshot of 'nl', we have everything translated.

Looks like a bug in our download script. I have filed bug 683469 to fix that.
No ETA on when we have all localizations. Interesting question on whether we intend to block on this feature being localized or not for release. I wouldn't block for beta, though.
In localizing selectAddons.dtd on Aurora, I stumbled upon the following:
Can somebody please explain me what the CSS unit "ch" is?

In line #1 : <!ENTITY upgrade.style               "width: 93ch; height: 448px;">
In line #13: <!ENTITY select.keep.style           "width: 6ch;">
In line #17: <!ENTITY select.action.style         "width: 35ch;">
In line #23: <!ENTITY select.name.style           "width: 33ch;">

Ref: http://mxr.mozilla.org/mozilla-aurora/source/toolkit/locales/en-US/chrome/mozapps/extensions/selectAddons.dtd
ch = character (simplifying a bit)
http://www.w3.org/TR/css3-values/#relative0
(In reply to flod (Francesco Lodolo) from comment #191)
> ch = character (simplifying a bit)
> http://www.w3.org/TR/css3-values/#relative0

Thanks Francesco - I hadn't met that unit before, but if it's in the standard, then there's nothing to worry about ;-D
(In reply to Søren Munk Skrøder from comment #192)
> (In reply to flod (Francesco Lodolo) from comment #191)
> > ch = character (simplifying a bit)
> > http://www.w3.org/TR/css3-values/#relative0
> 
> Thanks Francesco - I hadn't met that unit before, but if it's in the
> standard, then there's nothing to worry about ;-D

Just FYI - ch is the most usable unit for a font-dependent width, em has been used wronly for that in a number of places, but em is actually a font height unit, and the width:height ratio is not the same across different fonts. So, ch should always be used for widths that depend on font size, while em should be used for height that depend on font size.
I have updated the screenshots with hopefully all locales now:
http://people.mozilla.com/~hskupin/screenshots/select_addons/
(In reply to Henrik Skupin (:whimboo) from comment #194)
> I have updated the screenshots with hopefully all locales now:
> http://people.mozilla.com/~hskupin/screenshots/select_addons/

thanks, nl is there and i saw one small spelling error.
Depends on: 686658
Blocks: 354527
Depends on: 690714
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0

Verified on Ubuntu 11.04, Windows XP and 7 (upgrading from Firefox 7.0.1 to Firefox 8Beta1) using user installed add-ons and third party add-ons:
Ubuntu Firefox Modifications (Ubuntu)
AVGSafe Search 9.0.0.911 (W7)
DivX Plus Web Player HTML5 <video> 2.12.126 (W7)
Avg Safe search 9.0.0.911 (XP)
Java Quick starter 1.0 (XP)

The state and description of each addon is properly described.

There are two issues however:

Upgrading from Firefox 7 to Firefox 8 displays the select add-on dialog correctly, however, upgrading in the same fashion from Firefox 8 to 9 and from 9 to 10 does not (third party and user add-ons installed). There are a lot of comments here and dependencies, maybe I missed something somehow. Could anyone clarify?

If an add-on (third party or not) is incompatible with Firefox 7, the select add-on dialog will not list any action next to it when upgrading to Firefox 8. Is this expected as default?


Apart from this, could anyone provide a Mac OS third party add-on example-an application that installs a third party add-on?
(In reply to Virgil Dicu [QA] from comment #196)
> Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
> 
> Verified on Ubuntu 11.04, Windows XP and 7 (upgrading from Firefox 7.0.1 to
> Firefox 8Beta1) using user installed add-ons and third party add-ons:
> Ubuntu Firefox Modifications (Ubuntu)
> AVGSafe Search 9.0.0.911 (W7)
> DivX Plus Web Player HTML5 <video> 2.12.126 (W7)
> Avg Safe search 9.0.0.911 (XP)
> Java Quick starter 1.0 (XP)
> 
> The state and description of each addon is properly described.
> 
> There are two issues however:
> 
> Upgrading from Firefox 7 to Firefox 8 displays the select add-on dialog
> correctly, however, upgrading in the same fashion from Firefox 8 to 9 and
> from 9 to 10 does not (third party and user add-ons installed). There are a
> lot of comments here and dependencies, maybe I missed something somehow.
> Could anyone clarify?

This is a one-time only dialog. A user who has seen it once (the first time they run any version of Firefox since Firefox 8 with their profile) will never see it again. If you start Firefox 7 with a clean profile then upgrade directly to Firefox 9 then you should see the dialog. Going from 8 to 9 should not show it though.

> If an add-on (third party or not) is incompatible with Firefox 7, the select
> add-on dialog will not list any action next to it when upgrading to Firefox
> 8. Is this expected as default?

If it is incompatible with both 7 and 8 then the add-on was disabled in 7 and will still be disabled in 8 so there is no real change happening, so yes I'd expect to see no action there.

> Apart from this, could anyone provide a Mac OS third party add-on example-an
> application that installs a third party add-on?

I cannot name one for OSX, but if you extract an extension to ~/Library/Application Support/Mozilla/Extensions/{ec8030f7-c20a-464f-9b0e-13a3a9e97384}/<extension-id>/ then that is considered to be one of the third-party locations on OSX.
Depends on: 693698
Depends on: 694575
I just wanted to report that the dialog is not usable on Win XP with 768x1024 portrait. The dialog is too wide and scrollbars are missing.
Had this problem with both the latest Firefox and Thunderbird update.
(In reply to David Császár from comment #198)
> Created attachment 577017 [details]
> Screenshot of clipped dialog
> 
> I just wanted to report that the dialog is not usable on Win XP with
> 768x1024 portrait. The dialog is too wide and scrollbars are missing.
> Had this problem with both the latest Firefox and Thunderbird update.

David, thanks for the report. This could be a problem with just the German translation, could you file a separate bug in the "Mozilla Localizations" product, German localization? Thanks.
Filed Bug 705417
Attachment #549966 - Flags: ui-review?(jboriss)
You need to log in before you can comment on or make changes to this bug.