Closed Bug 397277 Opened 14 years ago Closed 13 years ago

Make /suite build with --enable-libxul

Categories

(SeaMonkey :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: standard8, Assigned: neil)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached patch The fix (obsolete) — Splinter Review
I've been playing around with building SeaMonkey with --enable-libxul, just to find out what else is required for this to happen (bug 394502) apart from the obvious mailnews conversion.

Some of the problems were in /suite where although we already compile with the external API, we don't compile with libxul. I checked what FF did, and there's some simple conversions required.

Patch attached with those conversions, this will allow /suite to build with --enable-libxul (or without it). I'll comment on bug 394502 in a while with the current instructions to allow a basic SeaMonkey build (only partially working of course) with libxul.
Attachment #282032 - Flags: review?(neil)
Comment on attachment 282032 [details] [diff] [review]
The fix

Why doesn't do_QueryElementAt work with libxul?

I think one of those enumerators could be changed to a singleton enumerator.
(In reply to comment #1)
> (From update of attachment 282032 [details] [diff] [review])
> Why doesn't do_QueryElementAt work with libxul?

Good question, I've just looked at mxr again, and do_QueryElementAt does seem to be defined in the xpcom glue: http://mxr.mozilla.org/seamonkey/source/xpcom/glue/nsArrayUtils.h#68

I was linking it in/with http://mxr.mozilla.org/seamonkey/source/suite/build/Makefile.in

and I was getting an error along the lines of "vtable undefined for nsQueryElementAt" (I can confirm this later if necessary).

I then (probably incorrectly) assumed that as some of the /browser code appeared to have been changed that the do_QueryElementAt wasn't available externally with --enable-libxul.

So it seems like we should be able to use it, but I don't know why the vtable isn't linking correctly.
From various discussions on irc, I need to talk to Benjamin about either implementing something like nsArrayUtils.h/.cpp for nsICollection in glue, or whether dropping nsISupportsArray from RDF (and everywhere else) is the better(?) option.

However, some instances of nsISupportsArray in /suite can be moved to nsI{Mutable,}Array without issue, so this patch does that.
Attachment #282032 - Attachment is obsolete: true
Attachment #282156 - Flags: review?(neil)
Attachment #282032 - Flags: review?(neil)
Attachment #282156 - Flags: review?(neil) → review+
Attachment #282156 - Attachment description: Move some of suite/profile from nsISupportsArray to nsI{Mutable,}Array → Move some of suite/profile from nsISupportsArray to nsI{Mutable,}Array (checked in)
To drop the do_QueryElementAt usage relating to nsISupportsArray for the bookmark code there are two options:

1) don't use RDF for the bookmark code
2) change RDF to use nsIArray rather than nsISupportsArray.

Either way, I'm not going to get to this in the near future and its not high priority, therefore reassign to default owner in case anyone else wants to pick this up.
Assignee: bugzilla → nobody
Depends on: 450781
Depends on: 450799
Depends on: mailnews-libxul
Depends on: 451871
No longer depends on: 450781
We were missing three symbols; I changed uses of NS_NewISupportsArray to do_CreateInstance and copied the code for NS_NewArrayEnumerator and do_QueryElementAt (unfortunately across repos so there's no history).
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #335196 - Flags: review?(bugzilla)
Attachment #335195 - Attachment is patch: true
Attachment #335195 - Attachment mime type: application/octet-stream → text/plain
Attachment #335195 - Attachment description: Copy parts of nsSupportsARra → Copy parts of nsSupportsARra [Dupe of next patch]
Attachment #335195 - Attachment is obsolete: true
No longer depends on: mailnews-libxul
Attachment #335196 - Flags: review?(bugzilla) → review+
Pushed changeset 325c0ff70623 to comm-central.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
(In reply to comment #8)
> Pushed changeset 325c0ff70623 to comm-central.

It looks like the new file is missing from the checkin !?
(In reply to comment #9)
> (In reply to comment #8)
> > Pushed changeset 325c0ff70623 to comm-central.
> It looks like the new file is missing from the checkin !?
Pushed changeset 00d766fa4563 to comm-central.

One day I'll get this right first time...
Attachment #335196 - Attachment description: Copy parts of nsSupportsArray.cpp → Copy parts of nsSupportsArray.cpp [Checkin: Comment 8+10]
Blocks: 451871
No longer depends on: 451871
You need to log in before you can comment on or make changes to this bug.