Visible delay in showing the "Language" category when opening the Add-Ons Manager

VERIFIED FIXED in mozilla2.0b4

Status

()

VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: bparr, Assigned: bparr)

Tracking

Trunk
mozilla2.0b4
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [AddonsRewrite])

Attachments

(1 attachment, 5 obsolete attachments)

(Assignee)

Description

9 years ago
If there is a language add-on installed, there is a noticeable delay between when the list of categories is first shown, and when the "Language" category is shown. This delay exists because the "Language" category should not be visible if there are no language packs installed (Bug 553483). The delay should be made less noticeable.
(Assignee)

Comment 1

9 years ago
Created attachment 456815 [details] [diff] [review]
Patch

Only show the list of categories after the hidden state for the Language and Search Engine categories is determined.
Assignee: nobody → bparr
Status: NEW → ASSIGNED
Attachment #456815 - Flags: review?(dtownsend)
Is that also flexible enough to make sure it behaves the same way for add-ons which overlay the add-ons manager and bring their own categories?
(Assignee)

Comment 3

9 years ago
Henrik, that should not be a problem. Assuming that the add-on added the category to the "categories" richlistbox, their category will be shown at the same time as the built-in categories.
Depending on if that will also happen lazily. Lets say Greasemonkey will add its own category only, if scripts are really installed. Otherwise it will also be hidden.
(Assignee)

Comment 5

9 years ago
Created attachment 456949 [details] [diff] [review]
Patch v2

Exposed variable and function to make it very easy to do what Henrik described in comment 4.

Steps:
1) Add onload listener to window object
2) When the onload listener is called, do "gCategories.pendingCategories++;" so that gCategories knows to wait for another category to initialize
3) Do work (possibly asynchronous work) to initialize category
4) Call "gCategories.notifyCategoryInitialized();" to let gCategories know that the category is initialized
Attachment #456815 - Attachment is obsolete: true
Attachment #456949 - Flags: review?(dtownsend)
Attachment #456815 - Flags: review?(dtownsend)
Thanks Ben! I wonder which parts can be automatically tested.
Flags: in-testsuite?
Flags: in-litmus?
(Assignee)

Comment 7

9 years ago
Created attachment 457043 [details] [diff] [review]
Patch and Test

Added a mochitest that tests that the category list only shows after all categories have initialized.
Attachment #456949 - Attachment is obsolete: true
Attachment #457043 - Flags: review?(dtownsend)
Attachment #456949 - Flags: review?(dtownsend)
(Assignee)

Comment 8

9 years ago
I think there is a better/simpler way to handle this bug. Instead of holding off showing the categories every time the Add-ons Manager is loaded, we could just persist the hidden state of the language category. So, basically all the time, there will not be a delay. There will still exist a delay when the user goes from not having language packs installed to having language packs installed, and vice versa. However, since this will happen very very rarely, it should be fine.

The main benefit of this different way is simplicity, but it also means that initially showing the list of categories will be slightly faster (not waiting for asynchronous calls to finish).

Overlaying the list of categories would be just like any other overlay that adds a richlistitem to a richlistbox, so no problems there.
(Assignee)

Comment 9

9 years ago
Created attachment 457256 [details] [diff] [review]
Patch and Test (new approach)

Uses persist approach described in comment 8. Therefore, it depends on Bug 571598. Also, the test depends on Bug 578580.
Attachment #457043 - Attachment is obsolete: true
Attachment #457256 - Flags: review?(dtownsend)
Attachment #457043 - Flags: review?(dtownsend)
(Assignee)

Updated

9 years ago
Depends on: 571598, 578580
Comment on attachment 457256 [details] [diff] [review]
Patch and Test (new approach)

This is an elegant solution, nice work. Since it depends on the other bug though I'm going to hold off reviewing it till after beta 2 madness is over
(Assignee)

Comment 11

8 years ago
Created attachment 458040 [details] [diff] [review]
Patch and Test v2

Updated test so it works with the landed patch for Bug 578580.
Attachment #457256 - Attachment is obsolete: true
Attachment #458040 - Flags: review?(dtownsend)
Attachment #457256 - Flags: review?(dtownsend)
(Assignee)

Comment 12

8 years ago
Created attachment 461491 [details] [diff] [review]
Patch and Test v3

Update patch now that Bug 571598 has been fixed.
Attachment #458040 - Attachment is obsolete: true
Attachment #461491 - Flags: review?(dtownsend)
Attachment #458040 - Flags: review?(dtownsend)
Attachment #461491 - Flags: review?(dtownsend) → review+
Attachment #461491 - Flags: approval2.0+
Fixed: http://hg.mozilla.org/mozilla-central/rev/41302fc3c620
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite?
Flags: in-testsuite+
Flags: in-litmus?
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
Verified with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4pre) Gecko/20100813 Minefield/4.0b4pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.