230965 - Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowInternal.scrollMaxY]" location: "JS frame :: chrome://messenger/content/mailWindowOverlay.js :: SpaceHit :: line 1757" data: no]

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, defect)

Description

[timeless]

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowInternal.scrollMaxY]"  nsresult:
"0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://messenger/content/mailWindowOverlay.js :: SpaceHit :: line 1757"  data: no]

This can happen when there's no view. the call should be wrapped in a try{}. if
it throws, then imo we should advance to the next message.

Comment 1

[James Ross]

Attachment: Wrap call in try block

This adds a try {} catch {} around the unsafe call, and if anything does go
wrong, it should try to move onto the next message.

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 139107 [details] [diff] [review]
Wrap call in try block


I guess that a 'r=?' is wanted !?

Comment 3

[[e-mail]]

Comment on attachment 139107 [details] [diff] [review]
Wrap call in try block

James, you seem to have forgotten the scroll up case. Also, I think an early
return on a successful scroll would result in neater code.

Comment 4

[Serge Gautherie (:sgautherie)]

(In reply to comment #3)
> (From update of attachment 139107 [details] [diff] [review])
> James, you seem to have forgotten the scroll up case. Also, I think an early

neil:
|contentWindow.scrollY > 0| should not throw exception, should it ?
Or is there something else to fix ?

> return on a successful scroll would result in neater code.

james:
|return| instead of |gotoNext = false|...

Comment 5

[[e-mail]]

(In reply to comment #4)
>|contentWindow.scrollY > 0| should not throw exception, should it ?
Ah, interesting that you should say that... I double checked, and it did use to
throw an exception, but because that caused bug 202206 a patch attached to bug
198987 was checked in to change the behaviour... roc, would a similar patch for
the Max versions be a suitable way to resolve this bug?

Comment 6

[James Ross]

(In reply to comment #3)
> (From update of attachment 139107 [details] [diff] [review])
> James, you seem to have forgotten the scroll up case. Also, I think an early
> return on a successful scroll would result in neater code.

I was just dealing with the stated error, though I should have spotted that code
and wrapped it too, sorry. Though from comment 5 patching this code may not be
needed/desirable.

(In reply to comment #4)
> james:
> |return| instead of |gotoNext = false|...

I know what an early return is...

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes! Just return NS_OK instead of NS_ERROR_FAILURE here:
http://lxr.mozilla.org/mozilla/source/dom/src/base/nsGlobalWindow.cpp#1965

Comment 8

[[e-mail]]

Attachment: roc's patch

I'm calling this roc's patch, not only because of comment 7, but because I got
the idea of it from roc's bug 198987 comment 19.

Which means that it ought to get someone else's r/sr...

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 147017 [details] [diff] [review]
roc's patch

Yeah, of course. r+sr=jst

Comment 10

[[e-mail]]

Comment on attachment 147017 [details] [diff] [review]
roc's patch

Trivial patch to stop throwing an unexpected exception.

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 147017 [details] [diff] [review]
roc's patch

a=asa (on behalf of drivers) for checkin to 1.7

Comment 12

[[e-mail]]

Fix checked in.