72556 - UI should not display busy cursor while content is loading

Status: NEW

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Christopher Blizzard (:blizzard)]

Right now the UI around a web page will show a busy cursor if you mouse over it
while the content in the web page is being loaded.  This doesn't make any sense
since the UI isn't busy, just the content area is.

Comment 1

[Christopher Blizzard (:blizzard)]

Attachment: patch

Comment 2

[Alec Flett]

looks ok to me.. r=alecf

Comment 3

[Scott MacGregor]

looks good to me too. r/sr=mscott

Comment 4

[Christopher Blizzard (:blizzard)]

stealing joki's bug

Comment 5

[James Green]

Comments: Works fine in all areas (linux opt), minor issues still to be solved:
1) has no effect in web browser while loading a page hovering over the
scrollbar. works fine over top and bottom toolbars + sidebar though. Scrollbar
still has the busy cursor.
2) has no effect when loading a newsgroup in mailnews.

Comment 6

[Christopher Blizzard (:blizzard)]

Attachment: all your cursor are belong to us

Comment 7

[Christopher Blizzard (:blizzard)]

There are some really bad assumptions made in the original code about the way
that cursors in mozilla work.  In mozilla, cursors are global for the entire
browser area.  That is, if you update the cursor in one widget no matter what
content the cursor is over it is updated.  This is why when a page is loading
the cursor is in a busy state even when it's over the UI buttons.  Also, when a
page was finished loading the cursor was always updated to the default cursor
even when the mouse was over, say, one of your bookmarks and it should have been
the hand cursor.

This patch creates a new mouse event called NS_MOUSE_UPDATE_CURSOR which when
sent via |nsIWidget::UpdateCursor| with an offset into that widget should update
the cursor since something has happened.  I've added support into
nsEventStateManager.cpp to handle this event.  This is called from the docshell
when the page is either started loading or finishes loading in case the pointer
is over the content area and needs to be changed from the busy cursor to the
default cursor or vice versa.

What is passed into that event is the result of something else that I've created
call the nsIPointerService.  This service allows you to query the pointer and
find out what widget it is over and what the offset of the pointer is in that
widget.  I've done an nsBasePointerService implementation which should return
nothing on platforms that don't have it implemented and have done an
implementation for X11.  Please keep in mind that the function that gets the
widget for unix is extra ordinarily expensive so it should only be used every
once in a while like at the end of a page load.

Someone needs to do feature work for at least the mac and win32 to get this
working on their platform.  It will work without specific support since the code
paths in the event state manager will handle the service missing or not
returning anything if it does work.

Comment 8

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

Instead of doing this "SetCursor" thing, why don't we just set "cursor: wait" on 
the viewport and let layout do its thing?

Comment 9

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

If you can't do this via layout somehow and you really need this patch, I ask 
this: why can't you just dispatch the mouse update event into the root widget 
for the docshell? The view manager should direct it to the correct frame.

Comment 10

[James Green]

I can't say I see much improvement with the latest patch. It could be that while
performing intensive operations it just isn't given the time to switch between
pointer states and thus I can see both 'busy' and 'normal' pointers when loading
things in mailnews hovering over the toolbar. Same can be said for loading web
pages hovering over the toolbar and sidebar. It certainly does not affect the
scrollbar.

Comment 11

[Christopher Blizzard (:blizzard)]

Attachment: latest patch

Comment 12

[Christopher Blizzard (:blizzard)]

This patch fixes at least one crash that I found while running this patch.

Comment 13

[Brendan Eich [:brendan]]

blizzard: how 'bout them roc questions?

/be

Comment 14

[Christopher Blizzard (:blizzard)]

Oh, I missed the second question from roc.  Sorry.

I don't think that it's possible to do what roc suggests because the view
manager doesn't know what the correct frame is.  The problem is that if you have
an asyncronus event that causes the cursor to have to change you don't know
where the cursor is so you can't route it to the right place.  Normally what
changes the cursor is a mouse move event.  It's got a specific target and offset
and that's how it's figured out what it should change the cursor to.

This code allows you to get the current mouse position and offset and has many
of the properties of the mouse move event but doesn't pass it on to the view
manager - it just updates the cursor.

Comment 15

[Madhur Bhatia]

QA contact updated

Comment 16

[Christopher Blizzard (:blizzard)]

Since the cursors landed in gtk this is much better.  However, at the end of
page loads the cursor still isn't updated properly.

Comment 17

[Christopher Blizzard (:blizzard)]

Moving out.