393842 - Move xpfe/components/search to suite/browser

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Mark Banner (:standard8)]

Now that bug 374862 has changed xpfe/components/search to frozen linkage, it makes it nice and easy for us to move and incorporate it into suite/common (its used from suite/common and suite/browser, so I think that suite/common is the best place for it).

Patches coming up later.

Comment 1

[Mark Banner (:standard8)]

Attachment: cvs moves

cvs moves file for this move.

Comment 2

[Mark Banner (:standard8)]

Attachment: Build changes for the move

Changes to apply for the move itself. Note there are no changes required to the files that are cvs moved.

Comment 3

[Robert Kaiser]

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

could we move this to suite/browser/ instead and also move the consumer from suite/common there? I'd think this is all browser-specific and we should do modularity correctly where it's easy enough.

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #3)
> (From update of attachment 278406 [details] [diff] [review])
> could we move this to suite/browser/ instead and also move the consumer from
> suite/common there? I'd think this is all browser-specific and we should do
> modularity correctly where it's easy enough.

Our only problem in doing this will be finding a way of updating panels.rdf in the user's profile to point to chrome://navigator... instead of chrome://communciator.

Its probably possible if I can find the right place/way of doing it.

Though I might split this up into two bugs/sections: one to move the cpp from xpfe to suite, and the second to reorganise search & bookmarks UI from suite/common to suite/browser - they both need doing, so it may be easier to divide the work up that way.

Comment 5

[Robert Kaiser]

Splitting it up sounds good to me if we just don't double-move files in the process and know what state we want to end up in :)

Comment 6

[[e-mail]]

(In reply to comment #4)
>Our only problem in doing this will be finding a way of updating panels.rdf
Do we migrate panels.rdf?

Where are the existing consumers anyway?

Comment 7

[Mark Banner (:standard8)]

(In reply to comment #6)
> (In reply to comment #4)
> >Our only problem in doing this will be finding a way of updating panels.rdf
> Do we migrate panels.rdf?

Currently no, but we probably will be when I get round to implementing bug 384175.

> Where are the existing consumers anyway?

I think its just items currently in suite/common/search (e.g. sidebarOverlay.js)

Comment 8

[[e-mail]]

(In reply to comment #7)
>>Where are the existing consumers anyway?
>I think its just items currently in suite/common/search (e.g. sidebarOverlay.js)
That's not in search ;-) Did you mean search-panel.xul ?

Comment 9

[Mark Banner (:standard8)]

(In reply to comment #8)
> (In reply to comment #7)
> >>Where are the existing consumers anyway?
> >I think its just items currently in suite/common/search (e.g. sidebarOverlay.js)
> That's not in search ;-) Did you mean search-panel.xul ?
> 

No I meant suite/common/sidebar/ and siderbarOverlay.js as you were asking the consumers of panels.rdf.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 278407 [details] [diff] [review]
Build changes for the move

Cancelling review requests as the code should really move to suite/browser.

Comment 11

[[e-mail]]

So it's just panels.rdf then?

Comment 12

[Mark Banner (:standard8)]

Attachment: cvs moves

From our previous discussions, looks like we can at least get the source moved for now till I work out about changing the rdf datasource.

Comment 13

[Mark Banner (:standard8)]

Attachment: Build changes for the move v2

Comment 14

[Robert Kaiser]

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

The target locations look good to me now.

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 279498 [details] [diff] [review]
Build changes for the move v2

I've just looked at this patch again, and its not quite right - its got a few things it shouldn't have and is missing a couple of others. Cancelling reviews, I'll attach a new patch later.

Note that the cvs moves file is still valid though.

Comment 16

[Mark Banner (:standard8)]

Attachment: Build changes for the move v3

This version is much better - includes the missing changes, also removes the redundant files from xpfe/components/search. Note I've not included the cvs moved files in this patch, they'll get removed at checkin time.

Comment 17

[Mark Banner (:standard8)]

Attachment: Follow up changes (checked in)

Fix the makefiles list and the installer. One day I'll get used to all the things we have change when we're moving stuff...

Comment 18

[Robert Kaiser]

Comment on attachment 280064 [details] [diff] [review]
Follow up changes (checked in)

Yes, that looks good to me, just wanted to ask you to clean up toolkit-makefiles.sh as well ;-)

Comment 19

[Mark Banner (:standard8)]

I've checked in the suite parts of this. I'll do the removals on Monday along with the toolkit-makefiles.sh change.

Comment 20

[Mark Banner (:standard8)]

Changing summary to reflect what we're actually doing...

Comment 21

[Mark Banner (:standard8)]

Comment on attachment 280064 [details] [diff] [review]
Follow up changes (checked in)

Requesting approval for checking in of the toolkit-makefiles.sh and xpfe/components/search changes (xpfe changes in attachment 279790 [details] [diff] [review]).

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 22

[Robert Kaiser]

Comment on attachment 280064 [details] [diff] [review]
Follow up changes (checked in)

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

Comment 23

[Mark Banner (:standard8)]

Comment on attachment 280064 [details] [diff] [review]
Follow up changes (checked in)

Cancelling approval request as received over irc per comment 22.

Comment 24

[Mark Banner (:standard8)]

All patches on this bug now checked in -> fixed. I'll deal with the UI moves in a different bug.