618502 - about:addons displays empty add-on page after uninstall/restart

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Jorge Villalobos [:jorgev] (he/him)]

Steps to reproduce:
0) Make sure you have Firefox setup to start with the last opened tabs.
1) Select an add-on in your list and click on the Remove button.
2) Right click on the add-on and select View More Information. The add-on details page appears, correctly indicating the add-on will be removed on restart.
3) Restart.

Observed result:
Firefox opens with about:addons showing an add-on detail page with no information.

Expected result:
The add-ons home should be shown instead. Nothing else will be good enough, IMO, because instead of Step 1, you can remove the add-on manually after closing Firefox, causing the same bug.

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

Hmm, with Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101210 Firefox/4.0b8pre i don't get the Details Page, but rather a "Loading" Info when i stop at Step 2. The State after Step 3 is the same.

I guess a Solution would be to not show resp. disable certain Menu Commands of "to be removed" marked Addon Entries, like "Show more Information" and "Disable". The "Remove" Menu Entry shold change to "Undo".

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Blair, that should be a dupe of bug 617291, right? Different path but same story.

Comment 3

[Dave Townsend [:mossop] (PTO until Aug 21st)]

Similar issues so there is some kind of dependency. This is the more likely case and so more likely to block, hopefully we'll just be fixing both cases at the same time though.

Comment 4

[Dave Townsend [:mossop] (PTO until Aug 21st)]

Blocks for a quick fix, I suggest just dropping to the default view when the expected add-on is missing

Comment 5

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #4)
> Blocks for a quick fix, I suggest just dropping to the default view when the
> expected add-on is missing

Agreed - just take it back to the default category

Comment 6

[Dave Townsend [:mossop] (PTO until Aug 21st)]

Attachment: patch rev 1

This makes us fall back to loading the default view if we cannot find the details for the add-on we want. It also makes it so the right category is selected if we have no previous view information (like if the add-ons manager is opened directly to a details view).

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 501696 [details] [diff] [review]
patch rev 1

>+  get selected() {
>+    return this.node.selectedItem ? this.node.selectedItem.value : null;
>+  },

I wonder if this can simplify some code in CategoryUtilities (followup fodder, if anything).


>+    // If the search category isn't selected then make sure the correct category
>+    // is
>+    if (gCategories.selected != "addons://search/")
>+      gCategories.select("addons://list/" + aAddon.type);

Nit: Can you re-wrap that comment, or something? That hanging "is" irks me. Also, the sentance is missing a fullstop.

>+  replaceView: function(aViewId) {
>+    if (aViewId == this.currentViewId)
>+      return;
>+
>+    gHistory.replaceState({
>+      view: aViewId,
>+      previousView: null

I'm assuming that because we only use |previousView| when |view| is the detail view, that setting |previousView| to null here won't have any side-effects. Correct?

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Also, that patch fixes bug 617291, in a round-about sort of way. Would be nice to show the addon with the changed ID, but at least this patch means the UI won't be broken when bug 617291 is encountered.

Comment 9

[Dave Townsend [:mossop] (PTO until Aug 21st)]

(In reply to comment #7)
> >+  replaceView: function(aViewId) {
> >+    if (aViewId == this.currentViewId)
> >+      return;
> >+
> >+    gHistory.replaceState({
> >+      view: aViewId,
> >+      previousView: null
> 
> I'm assuming that because we only use |previousView| when |view| is the detail
> view, that setting |previousView| to null here won't have any side-effects.
> Correct?

Correct. In fact the patch makes it so we don't technically need the previousView unless the previousView was a search, though having it just makes sure the right category gets selected even before the add-on details load.

Comment 10

[Dave Townsend [:mossop] (PTO until Aug 21st)]

Landed: http://hg.mozilla.org/mozilla-central/rev/546bfeae9ea5

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Following the steps from comment 0 we always end-up in opening the Get Add-ons page now. I think we should really open the accordingly list view instead. Can't we retrieve the type of the details view by the type attribute?

Comment 12

[Dave Townsend [:mossop] (PTO until Aug 21st)]

(In reply to comment #11)
> Following the steps from comment 0 we always end-up in opening the Get Add-ons
> page now. I think we should really open the accordingly list view instead.
> Can't we retrieve the type of the details view by the type attribute?

No because when the add-on no longer exists we don't know what type it was.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, which makes sense after the restart. Marking as verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20110109 Firefox/4.0b9pre ID:20110109030350