Closed Bug 393842 Opened 14 years ago Closed 14 years ago

Move xpfe/components/search to suite/browser

Categories

(SeaMonkey :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a1

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(3 files, 3 obsolete files)

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.
Attached patch cvs moves (obsolete) — Splinter Review
cvs moves file for this move.
Attachment #278406 - Flags: superreview?(neil)
Attachment #278406 - Flags: review?(kairo)
Attached patch Build changes for the move (obsolete) — Splinter Review
Changes to apply for the move itself. Note there are no changes required to the files that are cvs moved.
Attachment #278407 - Flags: superreview?(neil)
Attachment #278407 - Flags: review?(kairo)
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.
(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.
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 :)
(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?
(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)
(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 ?
(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.
Attachment #278406 - Flags: superreview?(neil)
Attachment #278406 - Flags: review?(kairo)
Comment on attachment 278407 [details] [diff] [review]
Build changes for the move

Cancelling review requests as the code should really move to suite/browser.
Attachment #278407 - Flags: superreview?(neil)
Attachment #278407 - Flags: review?(kairo)
So it's just panels.rdf then?
Attached patch cvs movesSplinter Review
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.
Attachment #278406 - Attachment is obsolete: true
Attachment #278407 - Attachment is obsolete: true
Attachment #279497 - Flags: superreview?(neil)
Attachment #279497 - Flags: review?(kairo)
Attached patch Build changes for the move v2 (obsolete) — Splinter Review
Attachment #279498 - Flags: superreview?(neil)
Attachment #279498 - Flags: review?(kairo)
Comment on attachment 279497 [details] [diff] [review]
cvs moves

The target locations look good to me now.
Attachment #279497 - Flags: review?(kairo) → review+
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.
Attachment #279498 - Attachment is obsolete: true
Attachment #279498 - Flags: superreview?(neil)
Attachment #279498 - Flags: review?(kairo)
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.
Attachment #279790 - Flags: superreview?(neil)
Attachment #279790 - Flags: review?(kairo)
Attachment #279790 - Flags: superreview?(neil) → superreview+
Attachment #279497 - Flags: superreview?(neil) → superreview+
Depends on: 395222
Attachment #279790 - Flags: review?(kairo) → review+
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...
Attachment #280064 - Flags: review?(kairo)
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 ;-)
Attachment #280064 - Flags: review?(kairo) → review+
I've checked in the suite parts of this. I'll do the removals on Monday along with the toolkit-makefiles.sh change.
Changing summary to reflect what we're actually doing...
Summary: Move xpfe/components/search to suite/common → Move xpfe/components/search to suite/browser
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.
Attachment #280064 - Flags: approval1.9?
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 on attachment 280064 [details] [diff] [review]
Follow up changes (checked in)

Cancelling approval request as received over irc per comment 22.
Attachment #280064 - Attachment description: Follow up changes. → Follow up changes (checked in)
Attachment #280064 - Flags: approval1.9?
All patches on this bug now checked in -> fixed. I'll deal with the UI moves in a different bug.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0alpha
You need to log in before you can comment on or make changes to this bug.