Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Hide version numbers for lightweight themes

VERIFIED FIXED in mozilla2.0b11

Status

()

Toolkit
Add-ons Manager
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: rdoherty, Assigned: sid0)

Tracking

Trunk
mozilla2.0b11
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

(Whiteboard: [softblocker])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

7 years ago
The add-ons manager shows the version number next to the add-on/theme name, which is fine for traditional add-ons, but lightweight themes don't have versions. There is effectively only one version of the lightweight theme. 

The version # that getpersonas.com supplies is the last modified timestamp. It's used to determine if there's an update. It should not be displayed as it's not useful for users.

http://grab.by/5vT5
Hardware: x86 → All
It 's a highly visible ui issue. As Ryan said we shouldn't confuse users here. Asking for blocking fx4.
blocking2.0: --- → ?
Assignee: nobody → bparr
blocking2.0: ? → betaN+
Assignee: bparr → mano
Status: NEW → ASSIGNED
Hrm, I don't see the version number in my build after installing a lightweight theme.  However, i do see an empty version field in the details view.
(Reporter)

Comment 3

7 years ago
(In reply to comment #2)
> Hrm, I don't see the version number in my build after installing a lightweight
> theme.  However, i do see an empty version field in the details view.

I still see them in nightlies. Have you tried installing a new persona from getpersonas.com?
This problem may only reveal itself after an update check is performed for the persona. We perform a check only for the active persona once a day.
Note: the fix here is held until some major theme-provider changes are landed.
Those changes will be made in bug 585339
Depends on: 585339
I mean bug 520124
Depends on: 520124
No longer depends on: 585339
No longer depends on: 520124
Created attachment 476590 [details] [diff] [review]
patch
Attachment #476590 - Flags: review?(dtownsend)
Comment on attachment 476590 [details] [diff] [review]
patch

>+function shouldShowVersionNumber(aAddon) {
>+  if (!aAddon.version)
>+    return false;
>+
>+  // The version number is hidden for lightweight themes.
>+  if (aAddon.type == "theme")
>+    return !/@personas\.mozilla\.org/.test(aAddon.id);
>+}

This looks like it should return true at the bottom.
Also, adding $ to the regexp probably wouldn't hurt.
Comment on attachment 476590 [details] [diff] [review]
patch

I agree with both of dao's comments. Could we get an automated test too?
Attachment #476590 - Flags: review?(dtownsend) → review-
Could you point me to some other tests for theme-type addons?
(In reply to comment #12)
> Could you point me to some other tests for theme-type addons?

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/browser/browser_bug562899.js is a test that uses a lightweight theme. You can probably get away with just simulating a normal theme using the MockProvider like most of the other browser chrome tests.
Whiteboard: [needs new patch]
Whiteboard: [needs new patch] → [needs new patch][eta: 22/12]
Whiteboard: [needs new patch][eta: 22/12] → [needs new patch][eta: 22/12][soft blocker]
Whiteboard: [needs new patch][eta: 22/12][soft blocker] → [needs new patch][eta: 22/12][softblocker]
Created attachment 506845 [details] [diff] [review]
patch addressing review comments + test

How's this?
Attachment #476590 - Attachment is obsolete: true
Attachment #506845 - Flags: review?(dtownsend)
(Assignee)

Updated

7 years ago
Whiteboard: [needs new patch][eta: 22/12][softblocker] → [needs review Mossop][softblocker]
Comment on attachment 506845 [details] [diff] [review]
patch addressing review comments + test

Looks good, thanks
Attachment #506845 - Flags: review?(dtownsend) → review+
Assignee: mano → sid.bugzilla
Keywords: checkin-needed
Whiteboard: [needs review Mossop][softblocker] → [softblocker]
Created attachment 507047 [details] [diff] [review]
patch + more tests

I wrote tests for the details view too. (There's also a typo fix in there -- addon -> aAddon).
Attachment #506845 - Attachment is obsolete: true
Attachment #507047 - Flags: review?(dtownsend)
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Whiteboard: [softblocker] → [needs review Mossop][softblocker]
Attachment #507047 - Flags: review?(dtownsend) → review+
Keywords: checkin-needed
Whiteboard: [needs review Mossop][softblocker] → [softblocker][has patch][can land]
http://hg.mozilla.org/mozilla-central/rev/a6a96e15c369
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [softblocker][has patch][can land] → [softblocker]
Target Milestone: --- → mozilla2.0b11
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b11pre) Gecko/20110127 Firefox/4.0b11pre by manually setting an older version number via about:config.
Status: RESOLVED → VERIFIED
Flags: in-litmus-
Duplicate of this bug: 632394
You need to log in before you can comment on or make changes to this bug.