382647 - Move xpfe Bookmarks source code to /suite

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Mark Banner (:standard8)]

Now that bug 378585 is fixed, we can move the bookmarks source code to /suite (Robert has already moved the chrome).

The logical place I think would be under /suite/browser, but chrome is already in suite/common.

So do we put the src under suite/browser/bookmarks and move the chrome across and update the uris (communicator->navigator)? Or do we just put the src with the chrome under suite/common/bookmarks/?

btw do we want suite/<whatever>/bookmarks/{public,src} directories as well?

Comment 1

[Robert Kaiser]

I'm not sure the public/src is the way to go, bsmedberg might be able to better tell that.
I think we should get browser-specific code into suite/browser (bookmarks and history, not sure what else we have).

Comment 2

[Mark Banner (:standard8)]

Attachment: cvs moves

Now that bookmarks is linking to external API, we can move it to /suite :-)

I'm putting this in suite/browser just like we did with the search code.

Here's the moves file, patch coming up in a mo.

Comment 3

[Mark Banner (:standard8)]

Attachment: Build patch

Comment 4

[Robert Kaiser]

Comment on attachment 280269 [details] [diff] [review]
cvs moves

The file locations look good, r=me :)

Comment 5

[[e-mail]]

Comment on attachment 280271 [details] [diff] [review]
Build patch

>Index: suite/browser/src/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/suite/browser/src/Makefile.in,v
>retrieving revision 1.10
>diff -u -p -r1.10 Makefile.in
>--- suite/browser/src/Makefile.in	7 Sep 2007 14:25:03 -0000	1.10
>+++ suite/browser/src/Makefile.in	9 Sep 2007 20:57:13 -0000
>@@ -57,14 +57,18 @@ REQUIRES = \
> 	gfx \
> 	layout \
> 	necko \
>+	nkcache \
> 	pref \
> 	rdf \
> 	uconv \
> 	intl \
>-	appcomps \
>+	txmgr \
>+	chardet \
>+	locale \
> 	$(NULL)

>Index: suite/build/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/suite/build/Makefile.in,v
>retrieving revision 1.6
>diff -u -p -r1.6 Makefile.in
>--- suite/build/Makefile.in	7 Sep 2007 14:25:04 -0000	1.6
>+++ suite/build/Makefile.in	9 Sep 2007 20:57:13 -0000
>@@ -62,7 +62,9 @@ REQUIRES	= \
> 		windowwatcher \
> 		suitebrowser \
> 		suitemigration \
>-		appcomps \
>+		nkcache \
>+		txmgr \
>+		chardet \
> 		$(NULL)
There's some inconsistencies creeping in here. I don't mind either way, but the first is that src/Makefile.in was almost alphabetical, and you added nkcache alphabetically but the other three you simply appended. The second is that one file requires locale and the other doesn't, although I guess that's because it's only used by nsBookmarksService.cpp and not nsBookmarksService.h?

Comment 6

[Robert Kaiser]

Comment on attachment 280271 [details] [diff] [review]
Build patch

yes, I like it, let's go for it. build.mk not referencing xpfe/ any more is really nice ;-)

r=me

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 280271 [details] [diff] [review]
Build patch

Requesting approval for checking in of the toolkit-makefiles.sh and xpfe/components/bookmarks changes.

This patch is all about moving code that's NPOTFFB away from xpfe into suite, but because of the current tree state, approval has to be requested for the build file changes where FF does use them.

Comment 8

[Mark Banner (:standard8)]

I've checked in the suite specific parts of this. I'm waiting on approvals or tree going a different colour before checking in the xpfe and toolkit parts.

Comment 9

[Robert Kaiser]

Comment on attachment 280271 [details] [diff] [review]
Build patch

I had a talk on IRC today, and got a=moconnor there for the toolkit-makefiles.sh hunk of this patch.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 280271 [details] [diff] [review]
Build patch

Cancelling review request per comment 9 as its been received over irc.

Comment 11

[Mark Banner (:standard8)]

All patches checked in -> fixed.