Closed
Bug 580298
Opened 12 years ago
Closed 12 years ago
Hide version numbers for lightweight themes
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
VERIFIED
FIXED
mozilla2.0b11
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: rdoherty, Assigned: rain1)
References
Details
(Whiteboard: [softblocker])
Attachments
(1 file, 2 obsolete files)
7.77 KB,
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
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
![]() |
||
Updated•12 years ago
|
Hardware: x86 → All
Comment 1•12 years ago
|
||
It 's a highly visible ui issue. As Ryan said we shouldn't confuse users here. Asking for blocking fx4.
blocking2.0: --- → ?
Updated•12 years ago
|
Assignee: nobody → bparr
blocking2.0: ? → betaN+
Updated•12 years ago
|
Assignee: bparr → mano
Status: NEW → ASSIGNED
Comment 2•12 years ago
|
||
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•12 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?
Comment 4•12 years ago
|
||
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.
Comment 5•12 years ago
|
||
Note: the fix here is held until some major theme-provider changes are landed.
Comment 7•12 years ago
|
||
I mean bug 520124
Comment 8•12 years ago
|
||
Attachment #476590 -
Flags: review?(dtownsend)
Comment 9•12 years ago
|
||
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.
Comment 10•12 years ago
|
||
Also, adding $ to the regexp probably wouldn't hurt.
Comment 11•12 years ago
|
||
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-
Comment 12•12 years ago
|
||
Could you point me to some other tests for theme-type addons?
Comment 13•12 years ago
|
||
(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.
Updated•12 years ago
|
Whiteboard: [needs new patch]
Updated•12 years ago
|
Whiteboard: [needs new patch] → [needs new patch][eta: 22/12]
Updated•12 years ago
|
Whiteboard: [needs new patch][eta: 22/12] → [needs new patch][eta: 22/12][soft blocker]
Updated•12 years ago
|
Whiteboard: [needs new patch][eta: 22/12][soft blocker] → [needs new patch][eta: 22/12][softblocker]
Assignee | ||
Comment 14•12 years ago
|
||
How's this?
Attachment #476590 -
Attachment is obsolete: true
Attachment #506845 -
Flags: review?(dtownsend)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [needs new patch][eta: 22/12][softblocker] → [needs review Mossop][softblocker]
Comment 15•12 years ago
|
||
Comment on attachment 506845 [details] [diff] [review] patch addressing review comments + test Looks good, thanks
Attachment #506845 -
Flags: review?(dtownsend) → review+
Updated•12 years ago
|
Assignee: mano → sid.bugzilla
Keywords: checkin-needed
Whiteboard: [needs review Mossop][softblocker] → [softblocker]
Assignee | ||
Comment 16•12 years ago
|
||
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•12 years ago
|
Keywords: checkin-needed
Whiteboard: [softblocker] → [needs review Mossop][softblocker]
Updated•12 years ago
|
Attachment #507047 -
Flags: review?(dtownsend) → review+
Updated•12 years ago
|
Keywords: checkin-needed
Whiteboard: [needs review Mossop][softblocker] → [softblocker][has patch][can land]
Comment 17•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a6a96e15c369
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [softblocker][has patch][can land] → [softblocker]
Target Milestone: --- → mozilla2.0b11
Comment 18•12 years ago
|
||
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-
You need to log in
before you can comment on or make changes to this bug.
Description
•