163690 - Find dialog throws assertion when no word found.

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[kinmoz]

In a TRUNK Mozilla build, any time you use find in a document to search for a
word that doesn't exist, you will hit the following assertion when the find
dialog tries to bring up the dialog that displays "The text you entered was not
found.":


###!!! ASSERTION: chrome shouldn't be calling alert(), use the prompt service: '
!isChrome', file x:/mozilla/dom/src/base/nsGlobalWindow.cpp, line 2188
Break: at file x:/mozilla/dom/src/base/nsGlobalWindow.cpp, line 2188


To reproduce, just load about:blank in the browser and use the find dialog to
search for any word. This assertion is annoying on Win32 since asserts bring up
a modal dialog that you have to dismiss.

Here's the stack during the assertion:


NTDLL! 77fa018c()
nsDebug::Assertion(const char * 0x020373b8, const char * 0x020373ac, const char
* 0x02037380, int 2188) line 280 + 13 bytes
nsDebug::WarnIfFalse(const char * 0x020373b8, const char * 0x020373ac, const
char * 0x02037380, int 2188) line 397 + 21 bytes
GlobalWindowImpl::Alert(GlobalWindowImpl * const 0x063271cc, const nsAString &
{...}) line 2188 + 32 bytes
XPTC_InvokeByIndex(nsISupports * 0x063271cc, unsigned int 62, unsigned int 1,
nsXPTCVariant * 0x0012e924) line 106
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_METHOD) line 1994 + 42 bytes
XPC_WN_CallMethod(JSContext * 0x05996348, JSObject * 0x03ee5488, unsigned int 1,
long * 0x06c875e4, long * 0x0012ec00) line 1266 + 14 bytes
js_Invoke(JSContext * 0x05996348, unsigned int 1, unsigned int 0) line 838 + 23
bytes
js_Interpret(JSContext * 0x05996348, long * 0x0012fa40) line 2801 + 15 bytes
js_Invoke(JSContext * 0x05996348, unsigned int 2, unsigned int 2) line 855 + 13
bytes
js_InternalInvoke(JSContext * 0x05996348, JSObject * 0x03ee5488, long 66228984,
unsigned int 0, unsigned int 2, long * 0x06ac5c80, long * 0x0012fb68) line 930 +
20 bytes
JS_CallFunctionValue(JSContext * 0x05996348, JSObject * 0x03ee5488, long
66228984, unsigned int 2, long * 0x06ac5c80, long * 0x0012fb68) line 3431 + 31 bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x06b082b8, void * 0x03ee5488,
void * 0x03f292f8, unsigned int 2, void * 0x06ac5c80, int * 0x0012fc00, int 0)
line 1041 + 33 bytes
GlobalWindowImpl::RunTimeout(nsTimeoutImpl * 0x0632ef98) line 4586 + 84 bytes
GlobalWindowImpl::TimerCallback(nsITimer * 0x06ac5d50, void * 0x0632ef98) line 4937
nsTimerImpl::Fire() line 337 + 17 bytes
nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x013ba3b0) line 579
nsAppShell::Run(nsAppShell * const 0x018c10c8) line 156
nsAppShellService::Run(nsAppShellService * const 0x018cbcd0) line 452
main1(int 1, char * * 0x00277410, nsISupports * 0x00000000) line 1509 + 32 bytes
main(int 1, char * * 0x00277410) line 1873 + 37 bytes
mainCRTStartup() line 338 + 17 bytes

Comment 1

[Akkana Peck]

Attachment: use the prompt service

I don't actually own this dialog, but it's easier to fix the bug than find out
who else should take it. :-)

I think something like this should fix it (not sure about passing null for the
window title).	I haven't tested it yet, but I have to head in to the office
now so I'm attaching the patch so I can test it from there.

Comment 2

[Akkana Peck]

Yup, this works and cures the assert.  Seeking review.  Hey, Charley, could you
review this very simple JS fix?

Comment 3

[Charles Manske]

Comment on attachment 98939 [details] [diff] [review]
use the prompt service

r=cmanske

Comment 4

[kinmoz]

Comment on attachment 98939 [details] [diff] [review]
use the prompt service

If an error is ever thrown, isn't it possible that |promptService| could be
null, in which case the |alert()| call would cause another error?

Comment 5

[Akkana Peck]

Attachment: Put the whole thing inside the try

Kin: good point.  Question: what should I do if there's no prompt service?  In
this new patch, I put the whole thing inside the try clause, and if it fails I
have a dump().	But that won't help people running release builds.  Would it be
better if I just removed the whole try clause, and let it throw an exception?

Comment 6

[kinmoz]

Comment on attachment 99538 [details] [diff] [review]
Put the whole thing inside the try

I think this patch is good enough. If you allowed the exception to be thrown
outside this method, we wouldn't execute the code below it, and there is no
guarantee that the user will get any feedback anyways from the method that
ultimately catches the exception.

Comment 7

[kinmoz]

Comment on attachment 99538 [details] [diff] [review]
Put the whole thing inside the try

btw, I just noticed that you are actually using dialog.bundle.getString() in
your dump ... can that throw an exception, if so, perhaps we should just forget
about translating and hard code an error message.

Comment 8

[Akkana Peck]

Thanks!  Fixed.  I did take out the dialog.bundle.getString() ... Kin's right
that it's probably safer just using a hardcoded string.