nsExtensionManager.js should get the XPCOM ABI and OS_TARGET from nsIXULRuntime instead of preprocessing

RESOLVED FIXED in mozilla1.8final

Status

()

Toolkit
Add-ons Manager
P1
blocker
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: Benjamin Smedberg, Assigned: Gavin)

Tracking

({fixed1.8.0.2, fixed1.8.1})

1.8.0 Branch
mozilla1.8final
fixed1.8.0.2, fixed1.8.1
Points:
---
Bug Flags:
blocking1.8.0.2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nvn-dl])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
This is needed for universal binaries of Firefox/Thunderbird which is targeted at Firefox 1.5.0.2...

The extension manager is currently preprocessed with a hardcoded value of TARGET_XPCOM_ABI... for mac universal builds this needs to change because we will be shipping the same nsExtensionManager.js for both mac-ppc and mac-intel. The extension manager should get nsIXULRuntime.XPCOMABI and use that value consistently.

(From bug 323328).
(Reporter)

Comment 1

12 years ago
Robert, if you could give me an ETA for this when you see it, we're going to need to jump on it quickly.
Severity: normal → blocker
Priority: -- → P1
Target Milestone: --- → Firefox1.5
Created attachment 208406 [details] [diff] [review]
patch

Tested an extension with a non-matching targetPlatform, and that a targetPlatform of <OS>_unknownABI won't match anything (by setting gXPCOMABI = "unknownABI" explicitly). Also tested that the XPCOMABI is properly substituted into the updateURL.
Assignee: robert.bugzilla → gavin.sharp
Status: NEW → ASSIGNED
Attachment #208406 - Flags: review?(robert.bugzilla)
(In reply to comment #1)
> Robert, if you could give me an ETA for this when you see it, we're going to
> need to jump on it quickly.
The patch from Gavin looks good but I'm going to hold off on reviewing it until after my head has cleared up some more (e.g. I have the flu) so I expect this weekend.
(Reporter)

Comment 4

12 years ago
Comment on attachment 208406 [details] [diff] [review]
patch

Please also remove the hunk at http://lxr.mozilla.org/mozilla/source/toolkit/mozapps/extensions/src/Makefile.in#52
Created attachment 208526 [details] [diff] [review]
remove defines too
Attachment #208406 - Attachment is obsolete: true
Attachment #208526 - Flags: review?(robert.bugzilla)
Attachment #208406 - Flags: review?(robert.bugzilla)
Comment on attachment 208526 [details] [diff] [review]
remove defines too

Gavin - looks good. Would you mind removing the exapnsion of __OS_TARGET__ as well? It would be a good thing to just take care of it as well in this bug. If you prefer either you or I can take care of it in another bug.
Attachment #208526 - Flags: review?(robert.bugzilla) → review+
Created attachment 208773 [details] [diff] [review]
remove OS_TARGET expansion as well

Robert/Benjamin: would appreciate a second look to make sure I didn't miss anything :).
Attachment #208526 - Attachment is obsolete: true
Attachment #208773 - Flags: review?(robert.bugzilla)
Comment on attachment 208773 [details] [diff] [review]
remove OS_TARGET expansion as well

Thanks
Attachment #208773 - Flags: review?(robert.bugzilla) → review+
Checked in on the trunk. I guess this should go on both branches, per comment 0?

mozilla/toolkit/mozapps/extensions/src/Makefile.in;                 1.6;
mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in;  1.166;
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Attachment #208773 - Flags: approval1.8.1?
Attachment #208773 - Flags: approval1.8.0.2?

Updated

12 years ago
Blocks: 323355
Summary: nsExtensionManager.js should get the XPCOM ABI from nsIXULRuntime instead of preprocessing → nsExtensionManager.js should get the XPCOM ABI and OS_TARGET from nsIXULRuntime instead of preprocessing
(Reporter)

Comment 10

12 years ago
Comment on attachment 208773 [details] [diff] [review]
remove OS_TARGET expansion as well

Two-week waiting period for the 1.8.0 branch is almost done.
Attachment #208773 - Flags: approval1.8.1? → branch-1.8.1+
Landed on the 1.8 branch.
mozilla/toolkit/mozapps/extensions/src/Makefile.in;                 1.5.4.1;
mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in; 1.144.2.19;
Keywords: fixed1.8.1
Flags: blocking1.8.0.2+
Comment on attachment 208773 [details] [diff] [review]
remove OS_TARGET expansion as well

approved for 1.8.0 branch, a=dveditz
Attachment #208773 - Flags: approval1.8.0.2? → approval1.8.0.2+
Checked in on the 1.8.0 branch.
mozilla/toolkit/mozapps/extensions/src/Makefile.in;               1.5.12.1;
mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in;  1.144.2.16.2.1;
Keywords: fixed1.8.0.2
Version: Trunk → 1.5.0.x Branch

Updated

12 years ago
Whiteboard: [nvn-dl]
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.