Remove extension manager dependency on classic.jar

RESOLVED FIXED in mozilla1.9.3a1

Status

()

defect
--
minor
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

Trunk
mozilla1.9.3a1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

Attachments

(4 attachments)

The extension manager currently relies on the application including a classic.jar containing the icon and preview images for the default theme. There isn't really any need for this we should just put the images alongside the theme's install.rdf just like for other themes.
Assignee

Comment 1

10 years ago
This patch stops packaging the images in classic.jar and installs them to <installdir>/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd} to match other themes. This is the only part that blocks bug 468011.

I think you're the right reviewer for this dao, but pass it on if not please.
Attachment #393354 - Flags: review?(dao)
Assignee

Comment 2

10 years ago
This stops us looking in classic.jar if the images aren't found in the filesystem.
Attachment #393355 - Flags: review?(robert.bugzilla)

Updated

10 years ago
Assignee

Comment 3

10 years ago
These are the changes for Thunderbird, as usual I can't guess who would be the right reviewer.
Attachment #393364 - Flags: review?(philringnalda)
(In reply to comment #3)
> These are the changes for Thunderbird, as usual I can't guess who would be the
> right reviewer.

For the record, the applicable review pages for comm-central can be found listed on the comm-central devmo page:

https://developer.mozilla.org/en/comm-central#comm-central_tree_rules

Having said that, philor is a good choice in this case.
Comment on attachment 393364 [details] [diff] [review]
thunderbird patch [checked in]

Looks good to me, other than the two bugs I had to file after actually looking at the previews :)

I don't see quite why we need := for the assignment to FILES, but then I don't see quite why we use it in several other similar places in the tree, so I guess it's fine by me either way.
Attachment #393364 - Flags: review?(philringnalda) → review+
Attachment #393354 - Flags: review?(dao) → review+
Comment on attachment 393355 [details] [diff] [review]
extension manager changes

Looks fine. Do you think it is worthwhile to add a test for this? Also, SeaMonkey and Sunbird will need similar changes as Thunderbird.
They don't, actually - as Mossop explained to me when I was worried about needing 1.9.1 ifdefs, SeaMonkey already does it this way, while Sunbird does it in calendar/sunbird/app/profile/extensions/{972ce4c6-7e08-4474-a285-3208198ce6fd}/Makefile.in
Cool that they are ahead of the curve on this one. :)
Assignee

Comment 9

10 years ago
Comment on attachment 393354 [details] [diff] [review]
build config changes [checked in]

Landed this part: http://hg.mozilla.org/mozilla-central/rev/f434af8fee98
Attachment #393354 - Attachment description: build config changes → build config changes [checked in]
Assignee

Updated

10 years ago
Attachment #393355 - Flags: superreview?(benjamin)
Comment on attachment 393355 [details] [diff] [review]
extension manager changes

I guess since this affects other applications it requires SR. Benjamin this just stops looking in classic.jar for the default theme's icon and preview, applications must include them alongside the install.rdf now, which has been an option all along anyway
I'll do a post to dev.platform about this change before landing it.
Comment on attachment 393364 [details] [diff] [review]
thunderbird patch [checked in]

Landed the Thunderbird part: http://hg.mozilla.org/comm-central/rev/1bb0e6d70434
Attachment #393364 - Attachment description: thunderbird patch → thunderbird patch [checked in]
(In reply to comment #6)
> (From update of attachment 393355 [details] [diff] [review])
> Looks fine. Do you think it is worthwhile to add a test for this?

I'm not entirely sure what it would test. Maybe just try to get the iconURL for the default theme and check it maps to the filesystem?
Testing the iconURL should suffice. When Fennec first started testing for app update they didn't have all of the pre-requisites at that time which caused a couple of bugs to be filed.
Something like this?
Attachment #394004 - Flags: review?(robert.bugzilla)
Comment on attachment 394004 [details] [diff] [review]
testcase [checked in]

Landed the test: http://hg.mozilla.org/mozilla-central/rev/376b78fc7223
Attachment #394004 - Attachment description: testcase → testcase [checked in]

Updated

10 years ago
Attachment #393355 - Flags: superreview?(benjamin) → superreview+
Posted to m.d.platform, but since we've branched already I've just landed this on trunk immediately. Will apply for approval shortly.

http://hg.mozilla.org/mozilla-central/rev/e5d56aa2b693
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3
Target Milestone: mozilla1.9.3 → mozilla1.9.3a1
Comment on attachment 393355 [details] [diff] [review]
extension manager changes

This is required to stop us looking for a classic.jar in the installation folder. Notice has been posted to the newsgroup. Pretty much no risk here
Attachment #393355 - Flags: approval1.9.2?
Comment on attachment 393355 [details] [diff] [review]
extension manager changes

a192=beltzner
Attachment #393355 - Flags: approval1.9.2? → approval1.9.2+
You need to log in before you can comment on or make changes to this bug.