589613 - [SeaMonkey 2.1, mochitest-browser-chrome] browser_Services.js | Services.search exists

Status: VERIFIED FIXED

(Core :: General, defect)

Description

[Robert Kaiser]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1282499502.1282502435.6363.gz

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_Services.js | Services.search exists
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_Services.js | Services.search is an nsIBrowserSearchService

The test for Services.search should only be executed when OpenSearch is built.

Comment 1

[Serge Gautherie (:sgautherie)]

Unexpected, bug 553815 was supposed to fix (all the issue of) this test...

Comment 2

[Robert Kaiser]

My guess is that the test for the interface in the test now always succeeds as the components are registered differently.

Comment 3

[Justin Wood (:Callek)]

> My guess is that the test for the interface in the test now always succeeds as
> the components are registered differently.

yea, that would be the case... I'll have to dive back into this(In reply to comment #2)

Comment 4

[Serge Gautherie (:sgautherie)]

Fwiw,
http://brasstacks.mozilla.com/topfails/test/SeaMonkey?name=chrome://mochikit/content/browser/toolkit/content/tests/browser/browser_Services.js
"2010-05-27 13:34": when this test was fixed previously.
"2010-08-14 10:19": when it started failing again.

Comment 5

[Ian Neal]

The build based on http://hg.mozilla.org/mozilla-central/rev/7b9c03096aa1 and http://hg.mozilla.org/comm-central/rev/6e5632e4d2f0 fails
The build based on http://hg.mozilla.org/mozilla-central/rev/67eac5028fd4 and http://hg.mozilla.org/comm-central/rev/e05dc1486d50 does not fail

Which gives http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=67eac5028fd4&tochange=7b9c03096aa1 and http://hg.mozilla.org/comm-central/pushloghtml?fromchange=e05dc1486d50&tochange=6e5632e4d2f0

I'm guessing that it will be a m-c change that will have caused the issue, I guess it is http://hg.mozilla.org/mozilla-central/rev/5761fb5b379f is the culprit so cc'ing gavin.

Comment 6

[Ian Neal]

I'm guessing some of the new code should have some ifdefs round it?

Comment 7

[Robert Kaiser]

Hah, well, he made the IDL to be built even when the toolkit search service is not built - there you have it.

Comment 8

[Ian Neal]

Attachment: ifdef bits out patch v0.1

Patch to ifdef out toolkit search code if toolkit search is not enabled

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 472040 [details] [diff] [review]
ifdef bits out patch v0.1

>diff --git a/netwerk/base/public/Makefile.in b/netwerk/base/public/Makefile.in

>-		nsIRedirectResultListener.idl \
>-		nsIBrowserSearchService.idl \
>-		$(NULL)
>+		nsIRedirectResultListener.idl
>+
>+ifdef MOZ_TOOLKIT_SEARCH
>+XPIDLSRCS	+= nsIBrowserSearchService.idl
>+endif
>+
>+XPIDLSRCS	+= $(NULL)

This doens't look right - AFAIK $(NULL)'s only purpose is to serve as a dummy end marker to ensure that all entries end with "\", so that adding/removing entries doesn't require editing two lines. So you should just leave the $(NULL) at the end of the other block - as in e.g. http://mxr.mozilla.org/mozilla-central/source/embedding/browser/webBrowser/Makefile.in#75 . r=me with that fixed.

Comment 10

[Ian Neal]

Attachment: Only build search code when MOZ_TOOLKIT_SEARCH is enabled patch v0.1a [Backed out: Comment 14]

Changes since v0.1:
* Fixed Makefile.in as suggested by reviewer.

Requesting a= for simple, low risk patch that fixes an issue with moved code (and helps remove an orange on SM tests)

Comment 11

[Justin Wood (:Callek)]

Comment on attachment 472053 [details] [diff] [review]
Only build search code when MOZ_TOOLKIT_SEARCH is enabled patch v0.1a [Backed out: Comment 14]

Hrmn actually this should have implicit a+ as "NPOTB" (since Firefox builds with BrowserSearchService of course)

Comment 12

[Ian Neal]

Comment on attachment 472053 [details] [diff] [review]
Only build search code when MOZ_TOOLKIT_SEARCH is enabled patch v0.1a [Backed out: Comment 14]

http://hg.mozilla.org/mozilla-central/rev/2a216165e361

Comment 13

[Serge Gautherie (:sgautherie)]

V.Fixed, per
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1284130726.1284133745.30739.gz
Linux comm-central-trunk debug test mochitest-other on 2010/09/10 07:58:46

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Backed out because this caused bug 595509 (MOZ_TOOLKIT_SEARCH isn't AC_DEFINED).

https://hg.mozilla.org/mozilla-central/rev/84baf90b040c

Comment 15

[Justin Wood (:Callek)]

IanN, on Bug 595509 (attachment 474379 [details] [diff] [review]) I provided a fix. Wish it was taken instead of a backout though. But lets get this (back) in!

Comment 16

[Justin Wood (:Callek)]

err home network fail :/

the fix was adding DEFINES += -DMOZ_TOOLKIT_SEARCH in the makefile at docshell/base/ conditional on ifdef MOZ_TOOLKIT_SEARCH

I can attach the fix here later.

Comment 17

[Ian Neal]

Attachment: Only build search code when MOZ_TOOLKIT_SEARCH is fully defined patch v0.2 [Checked in: Comment 20]

As MOZ_TOOLKIT_SEARCH is not AC_DEFINED need to add it to the Makefile.in
Used test from bug 595509 to check without any patch applied (PASS), with old patch applied (FAIL), with this patch applied (PASS).

Comment 18

[Justin Wood (:Callek)]

Comment on attachment 474432 [details] [diff] [review]
Only build search code when MOZ_TOOLKIT_SEARCH is fully defined patch v0.2 [Checked in: Comment 20]

This Makefile change is what my "empty patch" was intended to be in the other bug.

Comment 19

[Robert Kaiser]

The ideal fix would be to get in bug 410613 on the SeaMonkey side :P

Comment 20

[Ian Neal]

Comment on attachment 474432 [details] [diff] [review]
Only build search code when MOZ_TOOLKIT_SEARCH is fully defined patch v0.2 [Checked in: Comment 20]

http://hg.mozilla.org/mozilla-central/rev/da8dd922ba0c
a=NPOTB

Comment 21

[Serge Gautherie (:sgautherie)]

V.Fixed, per
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1284821028.1284823562.22253.gz
OS X 10.5 comm-central-trunk debug test mochitest-other on 2010/09/18 07:43:48