Add-on card in about:addons has update indicator (blue dot) when an unrelated add-on is installed
Categories
(Toolkit :: Add-ons Manager, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
thunderbird_esr68 | --- | unaffected |
thunderbird_esr78 | --- | wontfix |
firefox-esr68 | --- | unaffected |
firefox-esr78 | --- | wontfix |
firefox79 | --- | wontfix |
firefox80 | --- | wontfix |
firefox81 | --- | wontfix |
firefox84 | --- | wontfix |
firefox85 | --- | wontfix |
firefox86 | --- | fixed |
People
(Reporter: public, Assigned: A-NEUN, Mentored)
References
(Regression)
Details
(Keywords: good-first-bug, regression, Whiteboard: smoketestb78.2.0-pre)
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details |
In 78.2.0-pre build1 on Linux x64 (haven't tested other platforms yet), installing an add-on can cause other add-ons to get marked as 'updateable' (blue dot on the three-point button in the add-on tab, and an "Update" option in that menu).
STR:
- Install at least one add-on from ATO.
- Open the add-on tab in the category Extensions (you see a list of your add-ons).
- Use the "Find more add-ons" search bar at the top to search for another add-on in a new tab.
- Install another add-on and come back to the add-on tab
Expected:
The new add-on is visible, and no add-on has a blue dot / update menu entry.
Actual:
All add-ons other than the newly installed one have a blue dot / update menu entry. Reloading the add-on tab fixes the issue.
Reporter | ||
Comment 1•4 years ago
|
||
(This is unlikely to be a regression, the issue also occurs with 78.1.1; same STR)
Reporter | ||
Updated•4 years ago
|
Comment 2•4 years ago
|
||
Confirmed in 78.2.0 pre-build
Comment 3•4 years ago
|
||
See this for Firefox too? This code is pretty much shared (with only minor changes injected by Thunderbird)
Comment 4•4 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #3)
See this for Firefox too? This code is pretty much shared (with only minor changes injected by Thunderbird)
Confirmed for Firefox 79
Updated•4 years ago
|
Comment 5•4 years ago
|
||
Regressed by bug 1525178, specifically https://hg.mozilla.org/mozilla-central/rev/b570c6b5d91d
Before that patch addon install listeners were targeted to specific add-on cards.
After that patch every addon card will respond to every add-on install event, even if unrelated to the add-on.
These should ignore installs if it doesn't match this.addon
: https://searchfox.org/mozilla-central/rev/d54210d490ef335b13fc1fcac817525120c8c46b/toolkit/mozapps/extensions/content/aboutaddons.js#3306-3317
Similarly for https://searchfox.org/mozilla-central/rev/d54210d490ef335b13fc1fcac817525120c8c46b/toolkit/mozapps/extensions/content/aboutaddons.js#3319-3322 (but this is after 78, so not in Thunderbird).
Updated•4 years ago
|
Updated•4 years ago
|
Comment 7•4 years ago
|
||
This is relatively easy to fix, so I'll mark this as a good-first-bug and mentor this.
To get started, see comment 5 and https://wiki.mozilla.org/WebExtensions/Contribution_Onramp
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 9•4 years ago
|
||
I am pretty new to contributing to firefox and I am having a bit of trouble implementing this.
How should I compare it to this.addon
?
I was planning on comparing this.addon.id
and install.addon.id
, but install.addon
is null.
Comment 10•4 years ago
|
||
When is install.addon
null? (how did you test that?)
Have you tried checking .existingAddon
, to see if it has the expected value when .addon
is null?
The install
object is an AddinInstallWrapper
, instantiated here: https://searchfox.org/mozilla-central/rev/62c443a7c801ba9672de34c2867ec1665a4bbe67/toolkit/mozapps/extensions/internal/XPIInstall.jsm#1224-1262
and the class itself is declared here: https://searchfox.org/mozilla-central/rev/62c443a7c801ba9672de34c2867ec1665a4bbe67/toolkit/mozapps/extensions/internal/XPIInstall.jsm#2622-2732
Assignee | ||
Comment 11•4 years ago
|
||
Assignee | ||
Comment 12•4 years ago
|
||
(In reply to Rob Wu [:robwu] from comment #10)
When is
install.addon
null? (how did you test that?)
I tested that by placing a breakpoint in the Firefox JS Debugger, and checking if install.addon
matched this.addon
.
Have you tried checking
.existingAddon
, to see if it has the expected value when.addon
is null?
Thanks for this - it does.
Comment 13•4 years ago
|
||
Hey Tilden, Rob has left a review comment asking for a test: https://phabricator.services.mozilla.com/D90286
If you're stuck or unsure how to proceed, feel free to ask here or on Phabricator.
Assignee | ||
Comment 14•4 years ago
|
||
Thanks! I have just been a bit busy recently.
Comment 15•4 years ago
|
||
The issue appears in Firefox 76+, with the HTML sidebar.
Comment 16•4 years ago
|
||
Hey Tilden! It's been a while since we've heard from you - I wanted to check in on how this is going.
Assignee | ||
Comment 17•4 years ago
|
||
Thanks for checking up! I have had some quite stressful exams at school over the last month, but those are behind me now, so I should be able to commit myself to this again. (Thanks for the NI, I forgot about this for a bit). I think I have a good idea about how to proceed, but will ask for help if I require it.
Assignee | ||
Comment 18•4 years ago
|
||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 19•4 years ago
|
||
Hey Tilden! Just wanted to check in -- it looks like you're close to finishing this one off! Let us know if you'd like to keep working on it. :)
Assignee | ||
Comment 20•4 years ago
|
||
I will still be working on it. I've just had a period of mock exams, which the revision for prevented me from finishing this. Should be able to get it done pretty soon. Thanks for checking!
Comment 21•4 years ago
|
||
Comment 22•4 years ago
|
||
bugherder |
Updated•4 years ago
|
Comment 23•4 years ago
|
||
Tilden, congrats on landing this patch! 🎉 We've added your contribution to our recognition wiki at https://wiki.mozilla.org/Add-ons/Contribute/Recognition.
Hope to see you around the project in the future! 😎
Updated•4 years ago
|
Updated•4 years ago
|
Description
•