Closed Bug 341654 Opened 17 years ago Closed 16 years ago

search bookmarks broken on trunk if you do --disable-places


(Firefox :: Bookmarks & History, defect)

Not set



Firefox 3 alpha1


(Reporter: moco, Assigned: moco)




(2 files, 2 obsolete files)

search bookmarks broken on trunk if you do --disable-places.  (see related bug #336488)

note, I've already landed my fix for #336488 on the trunk.

the issue may be that we are not building localsearch service for firefox / trunk / disable places.
(In reply to comment #0)
> the issue may be that we are not building localsearch service for firefox /
> trunk / disable places.

Indeed, that is the issue.

I don't think that we want to fix this, since --disable-places is a temporary measure that was introduced to ease the landing of Places, and isn't officially supported.
we are planning to disable places very soon, so I'll go fix this ASAP.
Severity: minor → major
Target Milestone: --- → Firefox 3 alpha1
Attached patch patch (obsolete) — Splinter Review
note, for the 1.8 branch, this was done in mozilla/

ifneq (,$(filter browser suite,$(MOZ_BUILD_APP)))
tier_99_dirs += xpfe/components/search
Attachment #240054 - Flags: review?
Attachment #240054 - Flags: review? → review?(benjamin)
note, when I build xpfe/components/search on the Mac trunk, I get the following warnings:


/Users/sspitzer/Desktop/trunk-no-places/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp: In member function 'nsresult InternetSearchDataSource::AddSearchEngineInternal(const char*, const char*, const PRUnichar*, const PRUnichar*, nsIRDFResource*)':
/Users/sspitzer/Desktop/trunk-no-places/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp:2326: warning: enumeral mismatch in conditional expression: 'nsIInternetSearchContext::<anonymous enum>' vs 'nsIInternetSearchContext::<anonymous enum>'
/Users/sspitzer/Desktop/trunk-no-places/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp:2348: warning: enumeral mismatch in conditional expression: 'nsIInternetSearchContext::<anonymous enum>' vs 'nsIInternetSearchContext::<anonymous enum>'


/usr/bin/ld: warning multiple definitions of symbol _poll
../../../../dist/bin/libnspr4.dylib(unix.o) definition of _poll
/usr/lib/gcc/i686-apple-darwin8/4.0.1/../../../libSystem.dylib(poll.So) definition of _poll

note, I see the first warning on the branch.
Attachment #240054 - Flags: review?(vladimir)
fixed, using r=vlad.  (Would still like bsmedmerg's opinion on the change I made.)
Closed: 16 years ago
Resolution: --- → FIXED
So I'm not sure if this is the right fix now... this happens on gaius, it looks like xpfe/components/search depends on browser/components/bookmarks, but browser/components/bookmarks isn't even touched by this point?

/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/build/cygwin-wrapper ../../../../dist/bin/xpidl.exe -m typelib -w -I/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/public -I../../../../dist/idl -e _xpidlgen/nsISearchService.xpt -d .deps/nsISearchService.pp /cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/public/nsISearchService.idl
/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/build/cygwin-wrapper ../../../../dist/bin/xpt_link.exe _xpidlgen/search.xpt _xpidlgen/nsISearchService.xpt 
/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/build/cygwin-wrapper /home/cltbld/vc8-moztools/bin/nsinstall -m 644 _xpidlgen/search.xpt ../../../../dist/bin/components
make[7]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/public'
make[7]: Entering directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/src'
Building deps for nsInternetSearchService.cpp
/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/build/cygwin-wrapper cl -FonsInternetSearchService.obj -c  -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -DMOZILLA_INTERNAL_API -D_IMPL_NS_GFX -D_IMPL_NS_MSG_BASE -D_IMPL_NS_WIDGET  -DOSTYPE=\"WINNT5.2\" -DOSARCH=\"WINNT\" -DBUILD_ID=2006092521  -I../../../../dist/include/xpcom -I../../../../dist/include/string -I../../../../dist/include/rdf -I../../../../dist/include/necko -I../../../../dist/include/pref -I../../../../dist/include/uconv -I../../../../dist/include/intl -I../../../../dist/include/unicharutil -I../../../../dist/include/bookmarks -I../../../../dist/include   -I../../../../dist/include/appcomps -I../../../../dist/include/nspr          -GR- -TP -nologo -Zc:wchar_t- -W3 -Gy -FdnsInternetSearchService.pdb  -DNDEBUG -DTRIMMED -Zi -O1 -UDEBUG -DNDEBUG -MD            -D_CRT_SECURE_NO_DEPRECATE=1 -D_CRT_NONSTDC_NO_DEPRECATE=1 -DWINVER=0x500 -D_WIN32_WINNT=0x500 -DX_DISPLAY_MISSING=1 -DMOZILLA_VERSION=\"1.9a1\"!
_REGION_VERSION=\"1.9a1\" -DMOZILLA_SKIN_VERSION=\"1.8\"  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT /cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp
c:/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp(80) : fatal error C1083: Cannot open include file: 'nsIBookmarksService.h': No such file or directory
make[7]: *** [nsInternetSearchService.obj] Error 2
make[7]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search/src'
make[6]: *** [libs] Error 2
make[6]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components/search'
make[5]: *** [libs] Error 2
make[5]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe/components'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla/xpfe'
make[3]: *** [libs_tier_toolkit] Error 2
make[3]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla'
make[2]: *** [tier_toolkit] Error 2
make[2]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla'
make[1]: *** [alldep] Error 2
make[1]: Leaving directory `/cygdrive/c/builds/tinderbox/Fx-Trunk-Cairo/WINNT_5.2_Depend/mozilla'
make: *** [alldep] Error 2

Just backed this out; I'm not sure of the new toplevel bits, so I dunno where the fix should go...  I understand what the issue is better now, though!
Resolution: FIXED → ---
sorry about this vlad.

I wonder if I did something foolish like put search before bookmarks in the DIRS order?
in bug #336488 comment #13, gavin wrote:  "Firefox bookmarks search depends on code in xpfe/components/search".

looking at the ff2 build log to see how this is achieved.
Attachment #240054 - Attachment is obsolete: true
Attachment #240054 - Flags: review?(benjamin)
Probably blocking 1.9a1.
Flags: blocking1.9?
Whiteboard: [blocking1.9a1?]
for bug #317107, gavin implemented a new search service, so we don't need to build xpfe/components/search/src/nsInternetSearchService.cpp (the suite needs it, as it uses rdf:internetsearch, but firefox doesn't need it)

but for non-places bookmarks searching, we need the to build xpfe/components/search/src/nsLocalSearchService.cpp, as rdf:localsearch is still used.

so my plan is to build xpfe/components/search (if MOZ_PHOENIX and not PLACES), but not build nsInternetSearchService.cpp if MOZ_PHOENIX.

testing this now...
that's not going to work, because the XPCOM module stuff all lives in nsInternetSearchService.cpp.
Attached patch fix (obsolete) — Splinter Review
Attachment #240398 - Flags: review?(vladimir)
Comment on attachment 240409 [details] [diff] [review]
diff from one level higher, to include the change to mozilla/xpfe/components/

looks fine, thanks!
Attachment #240409 - Flags: review?(vladimir) → review+
I checked in the fix for this on 2006-09-28 12:07, but forgot to mark it fixed.
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060929 Minefield/3.0a1.
Flags: blocking1.9?
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [blocking1.9a1?]
This bug undid bug 288640... search should be built in application tiers, not toolkit.
Attachment #249382 - Flags: review?(
Comment on attachment 249382 [details] [diff] [review]
Fix build tiers, rev. 1

>Index: browser/

>+DIRS += xpfe/components/search

Should be tier_app_dirs.
Attachment #249382 - Flags: review?( → review+
build tier fix committed.
Was there a followup filed on the way attachment 249382 [details] [diff] [review] started Firefox-on-XULRunner burning, that I just can't find?
You need to log in before you can comment on or make changes to this bug.