Closed Bug 1175330 Opened 4 years ago Closed Last year

Version# in Detail view of Available update should be new version#

Categories

(Toolkit :: Add-ons Manager, defect)

40 Branch
defect
Not set

Tracking

()

RESOLVED INACTIVE
Tracking Status
firefox39 --- unaffected
firefox40 + wontfix
firefox41 - affected
firefox42 - affected

People

(Reporter: alice0775, Unassigned, Mentored)

References

Details

(Keywords: regression, uiwanted, Whiteboard: [qx:spec][good first bug])

[Tracking Requested - why for this release]:

Steps to Reproduce:
1. Open Addon Manager
2. Check for update
3. Switch to "Availale update" tab
4. Click "more" link of a update addon

Actual Results:
Version# in Detail view is old#, not new#

After landing Bug 1161183, there are no Version # in the List View
So, Version# in Detail view is important. New# should display.
Yes definitely. Maybe showing both old and new versions in some way would make sense here.
Flags: firefox-backlog+
Summary: Version# in Detail view of Availale update should be new version# → Version# in Detail view of Available update should be new version#
[Tracking Requested - why for this release]: Regression
Keywords: regression
Version: 41 Branch → 40 Branch
Adding a tracking flag for FF40, 41 and 42. Version# should be visible to users.
Dave or Dao can you help find an owner for this bug?
Flags: needinfo?(dtownsend)
Flags: needinfo?(dao)
There isn't anyone assigned to work on this feature right now and I don't think it makes sense to pull someone off other work for something of this low priority (it only affects users who manually update). I'd happily review a patch from the community for this, but it would be good to see if UX have suggestions for what this should look like.
Flags: needinfo?(dtownsend)
Flags: needinfo?(dao)
That makes sense. It does look low priority, but since it's a regression we are likely shipping in 40, I wanted to get someone potentially working on it for 42. 

Philipp, over to you.
Flags: needinfo?(philipp)
I marked it as QX so that a contributor can pick it up.
Flags: needinfo?(philipp)
Whiteboard: [qx:spec]
Not critical enough to track & updating the flags.
Duplicate of this bug: 1200390
Whiteboard: [qx:spec] → [qx:spec][good first bug]
(In reply to Dave Townsend [:mossop] from comment #5)
> There isn't anyone assigned to work on this feature right now and I don't
> think it makes sense to pull someone off other work for something of this
> low priority (it only affects users who manually update). I'd happily review
> a patch from the community for this, but it would be good to see if UX have
> suggestions for what this should look like.

I'm one of these "few" users who manually update, I already use the "Add-ons Manager - Version Number" extension to make sure add-on versions are displayed without the need for a mouseover, and I'd love it if this bug were FIXED. Now that it is labeled as [good first bug], I'm wondering if I could fix it myself. (Don't hope too much, but if I don't try I can't succeed.) In which source file should I look? Any idea of a mentor I might turn to if I have any questions? (I probably won't, I expect I'll either propose a first WIP patch or decide I'm not up to the task.)
Flags: needinfo?(dtownsend)
OS: Unspecified → All
Hardware: Unspecified → All
Version: 40 Branch → Trunk
Here is the code responsible for updating the detail view: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.js#2864
I'll happily answer any questions to have.
Flags: needinfo?(dtownsend)
(In reply to Dave Townsend [:mossop] from comment #11)
> Here is the code responsible for updating the detail view:
> https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/
> content/extensions.js#2864
> I'll happily answer any questions to have.

IIUC the { at the end of line 2864 is paired with the } at line 3079.

I'm not (yet?) taking this bug, but adding NI to self so it shows in my radar.

OK so IIUC this
2884     var version = document.getElementById("detail-version");
2885     if (shouldShowVersionNumber(aAddon)) {
2886       version.hidden = false;
2887       version.value = aAddon.version;
2888     } else {
2889       version.hidden = true;
2890     }
displays the version before updating. To fix this bug, we need to add somewhere the new version, which at this point the user hasn't yet decided to install or not.

This snippet of the same source file seems to do just that:
476       let tiptext = addonItem.getAttribute("name");
477 
478       if (addonItem.mAddon) {
479         if (shouldShowVersionNumber(addonItem.mAddon)) {
480           tiptext += " " + (addonItem.hasAttribute("upgrade") ? addonItem.mManualUpdate.version
481                                                               : addonItem.mAddon.version);
482         }
483       }
484       else {
etc.; IOW, when the add-on has the "upgrade" attribute, its mManualUpdate.version is the version of the proposed upgrade.

However, if we want to show both versions according to your comment #1, the fix for this bug cannot reuse the exact same (?:) operation as at lines 480-481, where it shows one or the other.

Mossop: between lines 2884 and 2885, would it be acceptable to expand the version just found, e.g. to "3.4.5 → 3.4.6" if the installed version is 3.4.5 and the version of the proposed upgrade is 3.4.6? I'm not sure whether or not this would have undesirable side-effects elsewhere, or even whether "→" (U+2192 RIGHT ARROW) is an acceptable character here. (Please also correct any error in my detective work shown above.)
Flags: needinfo?(dtownsend)
Flags: needinfo?(antoine.mechelynck)
P.S. The _updateView function is passed only the addon itself (equivalent to addonItem.mAddon at line 478) so it would need to access a sibling of its argument. I haven't yet decided how to do that: I imagine at least two ways to do it but I haven't decided which is more convenient.
_updateView is always called from within another function which only gets an Addon, never an AddonItem, and AFAICT one of them (onExternalInstall) is a public event, and the other (at lines 3095 sqq.) gets the addon by getAddonById. I don't know how to get back from an addon to its update version after all.

Sorry, Mossop, I'm too green for this: someone else will have to do it. :-(
Flags: needinfo?(dtownsend)
Flags: needinfo?(antoine.mechelynck)
Mentor: kmaglione+bmo
I would like to get started working on it if there is nobody else. Please give me direction
Mentor: kmaglione+bmo → aswan
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.