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

Status: RESOLVED FIXED

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

Description

[Benjamin Smedberg]

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).

Comment 1

[Benjamin Smedberg]

Robert, if you could give me an ETA for this when you see it, we're going to need to jump on it quickly.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: 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.

Comment 3

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

(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.

Comment 4

[Benjamin Smedberg]

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

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Attachment: remove defines too

Comment 6

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

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.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Attachment: remove OS_TARGET expansion as well

Robert/Benjamin: would appreciate a second look to make sure I didn't miss anything :).

Comment 8

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

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

Thanks

Comment 9

[:Gavin Sharp [email: [e-mail]]]

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;

Comment 10

[Benjamin Smedberg]

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.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

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;

Comment 12

[Daniel Veditz [:dveditz] back 2026-04-08]

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

approved for 1.8.0 branch, a=dveditz

Comment 13

[:Gavin Sharp [email: [e-mail]]]

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;