"Language" category should not be visible if there are no language packs installed

VERIFIED FIXED in mozilla1.9.3a5

Status

()

VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: Unfocused, Assigned: bparr)

Tracking

Trunk
mozilla1.9.3a5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [rewrite])

Attachments

(1 attachment)

"Language" category should not be visible if there are no language packs installed.
Depends on: 553517
Assignee: nobody → bmcbride
Flags: in-testsuite?
Flags: in-litmus?
Version: unspecified → Trunk

Comment 1

9 years ago
I think it would be cool if I could download more languages right from this panel, rather than having to the "get addons" panel to even find out Firefox supports different languages.
Keywords: uiwanted
"Languages" should indeed not show if the user has no language packs installed, but this shouldn't be uiwanted, so I'm removing the keyword.

(In reply to comment #1)
> I think it would be cool if I could download more languages right from this
> panel, rather than having to the "get addons" panel to even find out Firefox
> supports different languages.

While it's true that removing the panel means the fact that language packs are  available is hidden, this is acceptable because needing a language pack is so rare.  This is a user who wants to use a different language than the Firefox build they downloaded.  I don't feel it's necessary to trade screen real estate in the add-on manager in order to expose this feature to every user.
Keywords: uiwanted
blocking2.0: --- → final+
Assignee: bmcbride → bparr
(Assignee)

Comment 3

8 years ago
Created attachment 447666 [details] [diff] [review]
Patch

Also fixes Bug 566597.
Attachment #447666 - Flags: review?(dtownsend)
(Assignee)

Comment 4

8 years ago
I meant the patch also fixes Bug 567115.
Comment on attachment 447666 [details] [diff] [review]
Patch

You'll need to unregister the install listener when the page unloads (to avoid leaks and general weirdness). The rest looks good.

I wonder if we want to hide the category again if the last addon of that type is uninstalled without needing a restart? See bug 553494 and its temporary fix, bug 567120 - which will make restartless uninstalls immediately disappear from the list. I say "temporary" as in it probably won't get fixed properly until some time after 4.0 final. Not a big deal either way, I guess.
Attachment #447666 - Flags: review?(dtownsend) → review-
(Assignee)

Comment 6

8 years ago
I am confused. Doesn't gEventManager.shutdown() already uninstall all of gEventManager's listeners? Since we would only want to uninstall the listener when the page unloads, and because gEventManager.shutdown() is called when the page unloads, it seems redundant to uninstall the listener and then call gEventManager.shutdown().

I have looked at hiding the category if the last add-on is uninstalled. Since you can only add uninstall listeners for individual add-ons, the implementation gets pretty nasty. For example, the id of an add-on is not defined until onInstallEnded is fired. Originally, we were able to show the hidden category as soon as we knew the type of the installing add-on (onNewInstall or onInstallStarted depending on how the add-on is installed). So, in order to add a listener to the installing add-on, we would instead have to instead wait for onInstallEnded. I don't know if the gains from this feature are worth the drawbacks. First, categories will be hidden the next time the Add-ons Manager is loaded. Also, it seems that fixing Bug 553494 would make the feature useless since the category would not become empty until Restart (at very least show option to undo uninstall).
(In reply to comment #6)
> I am confused. Doesn't gEventManager.shutdown() already uninstall all of
> gEventManager's listeners? Since we would only want to uninstall the listener
> when the page unloads, and because gEventManager.shutdown() is called when the
> page unloads, it seems redundant to uninstall the listener and then call
> gEventManager.shutdown().

Eep, my bad. For some reason, I thought you were registering directly with AddonManager.



> I have looked at hiding the category if the last add-on is uninstalled. Since
> you can only add uninstall listeners for individual add-ons, the implementation
> gets pretty nasty. For example, the id of an add-on is not defined until
> onInstallEnded is fired. Originally, we were able to show the hidden category
> as soon as we knew the type of the installing add-on (onNewInstall or
> onInstallStarted depending on how the add-on is installed). So, in order to add
> a listener to the installing add-on, we would instead have to instead wait for
> onInstallEnded. I don't know if the gains from this feature are worth the
> drawbacks. First, categories will be hidden the next time the Add-ons Manager
> is loaded. Also, it seems that fixing Bug 553494 would make the feature useless
> since the category would not become empty until Restart (at very least show
> option to undo uninstall).

Yea, fair enough.
Attachment #447666 - Flags: review- → review+
Keywords: checkin-needed
Pushed as http://hg.mozilla.org/mozilla-central/rev/01f8d7c7654d
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100605 Minefield/3.7a5pre

Blair, can we have an automated test here? I think that would make more sense as having a Litmus test and we should be able to cover that, or am I wrong?
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9.3a5
Depends on: 571200
Agreed.
Flags: in-litmus? → in-litmus-
Tests were added as a part of bug 572561
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.