Closed Bug 378585 Opened 13 years ago Closed 13 years ago
Thunderbird/Composer/Sunbird shouldn't need to build in xpfe/components/bookmarks/public
Currently Thunderbird/Composer/Sunbird build in xpfe/components/bookmarks/public: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/xpfe/components/Makefile.in&rev=1.96&mark=49-58,122#49 SeaMonkey is the only app that builds xpfe/components/bookmarks completely (http://mxr.mozilla.org/mozilla/source/suite/build.mk#62). The bookmarks chrome is moving to /suite as part of bug 377799. It'd be nice if we could move the rest of it around the same time. From an mxr search (http://mxr.mozilla.org/mozilla/search?string=nsIBookmarksService) it appears that Thunderbird/Composer/Sunbird aren't using nsIBookmarksService even though they build the interface. The only thing this would affect is the building of extensions/metrics with Thunderbird/Composer/Sunbird - its not built for anyone by default, but does look used, but is it currently used for those applications? cc'ing folks who I think are all relevant to this bug. If we we're not allowed to move xpfe/components/bookmarks to suite because those apps still require it, then we'll have to wontfix this bug and create a new one to move the xpfe/components/bookmarks/public stuff to a more common area - which seeing as ff has its own interface specification, I don't think would be appropriate.
This was originally the case because nsSearchService require nsIBookmarksService. searchservice is now part of tier_app, and so we can (should) move xpfe/components/bookmarks to tier_app as well.
Stops Thunderbird/Sunbird/Composer building in xpfe/components/bookmarks/public. I can't see any reason why these still need to (ref also Benjamin's comment 1), and those apps don't appear to be including bookmarks.xpt in their deliveries so I think there are no other changes required. Requesting reviews from Thunderbird, Sunbird and Composer leads for this small change, that'll let us clean up the tree a bit more ;-)
Comment on attachment 262922 [details] [diff] [review] The fix excellent.
Attachment #262922 - Flags: superreview?(mscott) → superreview+
Comment on attachment 262922 [details] [diff] [review] The fix looks good for sunbird, r=mvl
Attachment #262922 - Flags: review?(mvl) → review+
actually, doesn't packages-static need to be updated? http://lxr.mozilla.org/seamonkey/source/calendar/installer/windows/packages-static#62 62 bin\components\bookmarks.xpt
(In reply to comment #5) > actually, doesn't packages-static need to be updated? > http://lxr.mozilla.org/seamonkey/source/calendar/installer/windows/packages-static#62 Yes sorry, I'd somehow missed that when I was searching. Sunbird already has it in the removed-files.in which is quite interesting really ;-)
Attachment #263145 - Flags: review?(mvl)
Comment on attachment 263145 [details] [diff] [review] Remove bookmarks.xpt from sunbird packaging yes. r=mvl
Attachment #263145 - Flags: review?(mvl) → review+
Comment on attachment 262922 [details] [diff] [review] The fix I've not been able to get a response from Daniel during the month this request has been set, therefore passing review to Benjamin to confirm this should be ok from the build config view (I was trying for approvals from all appropriate app owners).
Attachment #262922 - Flags: review?(daniel) → review?(benjamin)
Both patches checked in -> fixed.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Mark, we have a pretty strict cross-commit policy in mozilla/calendar for all changes that apply to both trunk and branch. Could you therefore please commit attachment 262922 [details] [diff] [review] also to the 1.8 branch, please. Thank you.
(In reply to comment #10) > Mark, we have a pretty strict cross-commit policy in mozilla/calendar for all > changes that apply to both trunk and branch. Could you therefore please commit > attachment 262922 [details] [diff] [review] also to the 1.8 branch, please. Thank you. I checked attachment 263145 [details] [diff] [review] in to branch last night (after checking with Simon that 263145 was the correct one not 262922).
You need to log in before you can comment on or make changes to this bug.