"You don't have any add-ons of this type installed" doesn't disappear as it should

VERIFIED FIXED in mozilla2.0b8

Status

()

defect
--
trivial
VERIFIED FIXED
9 years ago
4 years ago

People

(Reporter: tiago.morbus.sa, Assigned: bparr)

Tracking

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

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

()

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100828 Firefox/4.0b5pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100828 Firefox/4.0b5pre

I came across this when, for the first time ever (woohoo!) I installed an add-on on Minefield. And the screenshot I'll upload right away explains it pretty well.

Reproducible: Always

Steps to Reproduce:
1. Have no extensions installed
2. Install an extension
Actual Results:  
The "You don't have any add-ons of this type installed" doesn't disappear, and instead groups with the installed extension in an awkward way.

Expected Results:  
"You don't have any add-ons of this type installed" should disappear and give place to just the installed add-on.
Reporter

Comment 1

9 years ago
Here's the screenshot.
Reporter

Comment 2

9 years ago
Added McBride to the list so this doesn't get lost. Sorry for the bother.
Reporter

Updated

9 years ago
Blocks: 550048
Yes, it looks awkward, and yet it's true: until you restart, the add-on isn't yet "installed".
Component: General → Add-ons Manager
OS: Windows 7 → All
Product: Firefox → Toolkit
QA Contact: general → add-ons.manager
Hardware: x86 → All
Version: unspecified → Trunk
I think it's a dupe but cannot find the original one. Note that all list views are affected by this issue. See bug 591024. I would say lets track it separately for now.
Blocks: 590175
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [DUPEME]
blocking2.0: --- → ?
Yea, that notification should definitely not show.
blocking2.0: ? → betaN+
Whiteboard: [DUPEME]
Whiteboard: [good first bug]
Assignee

Comment 6

9 years ago
Going to look into this.
Assignee: nobody → bparr
Assignee

Comment 7

9 years ago
Posted patch Patch and Test (obsolete) — Splinter Review
Fixes cases where the hidden state of the empty notice should change but wasn't.

Also fixes problem where installs would show up in a List View that did not match their type (e.g. an extension install showing up when the Language category is showing). This was happening because the Add-ons Manager would add an item onNewInstall, even though it did not know the type of the install.
Attachment #480391 - Flags: review?(dtownsend)
(In reply to comment #7)
> Also fixes problem where installs would show up in a List View that did not
> match their type (e.g. an extension install showing up when the Language
> category is showing). This was happening because the Add-ons Manager would add
> an item onNewInstall, even though it did not know the type of the install.

Think that may fix bug 601459 too.
(In reply to comment #7)
> Also fixes problem where installs would show up in a List View that did not
> match their type (e.g. an extension install showing up when the Language
> category is showing). This was happening because the Add-ons Manager would add
> an item onNewInstall, even though it did not know the type of the install.

So where does this patch make thenm show up during download when the type is unknown? It sounds like this also fixes bug 571458 (yay!) except that we haven't decided what the right fix for that actually is yet.
Assignee

Comment 10

9 years ago
The current patch would not insert an installing add-on into the current view until it can match the add-on type with the current list type. So, the add-on would not show up if the type is unknown (e.g. when downloading).
Will review tomorrow after Boriss confirms the new behavior is good and just.
Whiteboard: [good first bug] → [good first bug][has patch][needs review Mossop]
Comment on attachment 480391 [details] [diff] [review]
Patch and Test

Sorry for the delays. This looks pretty good however it doesn't apply since bug 558134 made a couple of changes in this area. Could you update it and I'll get it reviewed pretty quick. Also adding a test verifying that no download of the wrong type shows up anymore would be useful.
Attachment #480391 - Flags: review?(dtownsend) → review-
Whiteboard: [good first bug][has patch][needs review Mossop] → [good first bug][needs new patch]
Ben, would you have time to update the patch?
Assignee

Comment 14

9 years ago
I unfortunately won't have time until next next Monday (12/6). I will have a good chunk of time to work on my bugs the week of that Monday though.
Assignee

Comment 15

9 years ago
Update patch so it can actually be applied.
Attachment #480391 - Attachment is obsolete: true
Attachment #496281 - Flags: review?(dtownsend)
Comment on attachment 496281 [details] [diff] [review]
Patch and Test v2

Thanks this looks great. I think I need to sit down and think harder about how available installs show up in the API after Firefox 4, it doesn't seem to be quite right yet but this patch seems to be the best way to handle it right now.

I did realise that we've always not sorted newly added items correctly but I filed bug 618396 on that, no need to solve that yet.
Attachment #496281 - Flags: review?(dtownsend) → review+
Whiteboard: [good first bug][needs new patch]
Darn, landed but I forgot to put it in your name, sorry

http://hg.mozilla.org/mozilla-central/rev/245e6ab9e2e3
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101212 Firefox/4.0b8pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.