562848 - Lightweight themes without descriptions display undefined for text (error console - Warning: reference to undefined property aTheme[prop])

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Some lightweight themes don't have descriptions. However, the Addon objects the LightweightThemeManager provides doesn't take this into account. When getting the description for such a lightweight theme, it tries to access an undefined property - resulting in a JS warning and the description getter returning undefined, instead of an empty string.


Warning: reference to undefined property aTheme[prop]
Source File:
file:///C:/Program%20Files%20(x86)/Minefield/modules/LightweightThemeManager.jsm
Line: 398

(Originally from bug 562765 comment 3)

Comment 1

[Robert Strong (they/them - no direct email)]

Attachment: patch rev1

Comment 3

[Robert Strong (they/them - no direct email)]

Dave, I went with the optional list in the global scope to decide which should be optional. It does seem that version should not be optional to me but I don't know the light weight themes implementation all that well.

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 442627 [details] [diff] [review]
patch rev1

Make the version property return "" when not set (according to toolkit version rules that is the same as "0" but won't display as badly I think). Also please use |prop in aTheme| as the test or we'll still emit strict JS errors I think.

Comment 5

[Robert Strong (they/them - no direct email)]

Attachment: patch - like so?

btw: it displays 'Version' without a version number for lightweight themes.

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 443727 [details] [diff] [review]
patch - like so?

Looks good thanks.

Comment 7

[Dave Townsend [:mossop]]

http://hg.mozilla.org/projects/addonsmgr/rev/dd337a8ff851

Comment 8

[Dave Townsend [:mossop]]

http://hg.mozilla.org/mozilla-central/rev/6884244e3f75

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Any chance to verify that fix? Blair, have you used a special persona? How can I easily find one which doesn't have a description?

Comment 10

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Sorry, I don't have any example persona for this (I don't think I ever saw this myself). Maybe Rob Strong can remember which persona he used?

Comment 11

[Robert Strong (they/them - no direct email)]

Clean Fox doesn't have a description
https://addons.mozilla.org/en-US/firefox/persona/60272

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100523 Minefield/3.7a5pre ID:20100523030649

Could we combine the Litmus test with the one on bug 562765? Would it be enough to test a personas only or do we also have to create an example XPI?

Comment 13

[Robert Strong (they/them - no direct email)]

You can - see bug 562765 comment #27

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

(In reply to comment #13)
> You can - see bug 562765 comment #27

Blair will work on an automated test. If it doesn't succeed I will hop-on and create a Mozmill test.