build 2003032404 Win XP 1 enable smoothcrolling : user_pref("general.smoothScroll", true); 2 go to Newsgroups and display a message 3 press the space bar to go down one screen expected result : message scrolls to the next part, if we are at the end of the message, hitting the space bar should jump to the next unread message in the newsgroup. actual result : message scrolls and jumps directly to the next message.

-> Mailnews

No. Smooth scrolling bugs are mine.

*** Bug 199242 has been marked as a duplicate of this bug. ***

*** Bug 200045 has been marked as a duplicate of this bug. ***

The problem is fairly simple. mailWindowOverlay.js has this SpaceHit function which does: > contentWindow.scrollByPages(numPages); > // if at the end (or start) of the message, go to the next one > if (oldScrollY == contentWindow.scrollY) { > goDoCommand(command); > } The problem is that with smooth scrolling, 'scrollByPages' doesn't actually move the scrollY right away. So this technique is no longer a good way to determine that we're scrolled to the bottom. We need to add a new property that tells us whether we're at the bottom or not.

This patch does three things: -- adds "scrollMaxX", "scrollMaxY" properties to nsGlobalWindow which return the maximum scroll offset the window can currently have (document width/height - window width/height) -- makes mailnews compare scrollY with scrollMaxY to see whether we're at the end of the message -- fix a smooth-scrolling bug where a smooth-scroll of a very small distance (e.g., 1 pixel) would fail because the 1 pixel would be turned into 10 frames of 1-2 twips movement each, each movement being rounded down to 0 pixels.

Comment on attachment 120357 [details] [diff] [review] fix this patch should be non-scary

Comment on attachment 120357 [details] [diff] [review] fix >Index: dom/public/idl/base/nsIDOMWindowInternal.idl Changes to this file need moa from jst. I'll mark r=, but please make sure to get sr from him. >+GlobalWindowImpl::GetScrollMaxXY(PRInt32* aScrollMaxX, PRInt32* aScrollMaxY) >+ nscoord xPos, yPos; >+ rv = view->GetScrollPosition(xPos, yPos); >+ NS_ENSURE_SUCCESS(rv, rv); Why do you need this in GetScrollMaxXY? It's not used... r=me with that extraneous code removed.

Oh yeah. I'll remove that. Thanks.

Comment on attachment 120357 [details] [diff] [review] fix - In nsIDOMWindowInternal.idl: + + /* The maximum offset that the window can be scrolled to + (i.e., the document width/height minus the scrollport width/height) */ + readonly attribute long scrollMaxX; + readonly attribute long scrollMaxY; Given that you're adding new readonly properties to all window objects here, please make them "replaceable" by adding checks for these strings to nsDOMClassInfo::IsReadonlyReplaceable(). sr=jst with that addition.

OK, but how come pageXOffset and pageYOffset aren't in that replaceable list?

This is what I'm checkin in.

*** Bug 199671 has been marked as a duplicate of this bug. ***

Maybe pageXOffset and pageYOffset should be 'replaceble' too, but so far they haven't caused any problems, and they've been around for a long time. These are new properties and they could cause existing scripts to break if they're not replaceable, that's why I wanted you to make them replaceable, and thanks for doing that. Patch looks good.

Checked in. Thanks all!

err, fixed

*** Bug 201059 has been marked as a duplicate of this bug. ***

This seems to have introduce some JS errors into Chatzilla, which may or may not be harmless (i.e. backing out this patch did not fix bug 202206).

OK, Neil, try this: In GlobalWindowImpl::GetScrollXY(PRInt32* aScrollX, PRInt32* aScrollY) change if (!view) return NS_ERROR_FAILURE; to if (!view) return NS_OK; That should fix it.

Yes, that change fixes those pageYOffset JS errors in Chatzilla.

Changes NS_ERROR_FAILURE to NS_OK as per comment #19

Comment on attachment 120951 [details] [diff] [review] Patch v1.0 to GetScrollXY trivial patch

Checked in.