Bug 131172 - Mouse Cursor doesn't change appropriately.
Summary: Mouse Cursor doesn't change appropriately.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL: http://emberjs.jsbin.com/nosap/1
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-03 10:56 PDT by Nathan Hammond
Modified: 2016-09-17 07:11 PDT (History)
10 users (show)

See Also:


Attachments
Patch (1.57 KB, patch)
2014-04-07 05:36 PDT, Miyoung Shin
no flags Details | Formatted Diff | Diff
Patch (4.18 KB, patch)
2014-04-11 05:02 PDT, Miyoung Shin
no flags Details | Formatted Diff | Diff
Patch (4.11 KB, patch)
2014-04-12 18:07 PDT, Miyoung Shin
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 (462.90 KB, application/zip)
2014-04-12 19:08 PDT, Build Bot
no flags Details
Patch (4.18 KB, patch)
2014-04-13 00:41 PDT, Miyoung Shin
no flags Details | Formatted Diff | Diff
Patch (7.25 KB, patch)
2014-07-15 08:02 PDT, Miyoung Shin
no flags Details | Formatted Diff | Diff
Patch (4.89 KB, patch)
2014-07-15 08:37 PDT, Miyoung Shin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nathan Hammond 2014-04-03 10:56:38 PDT
Visit: http://emberjs.jsbin.com/nosap/1 

Go to the bottom of the page, find the "NODES" link. Hover over it, move cursor off to the right of the element. Cursor will now remain as "pointer" until it intersects with a different element.

This likely has something to do with the applied transform.

In order to see this bug you MUST NOT have flexbox enabled without prefixes.
Comment 1 Miyoung Shin 2014-04-06 02:57:36 PDT
Is there anyone to follow this issue ?
If not, can I pick this one ?
Comment 2 Miyoung Shin 2014-04-07 05:36:08 PDT
Created attachment 228728 [details]
Patch
Comment 3 zalan 2014-04-08 07:42:55 PDT
Please add a test case.
Comment 4 Miyoung Shin 2014-04-08 08:08:13 PDT
do you mean new layout test case for this issue?
Comment 5 Miyoung Shin 2014-04-09 08:42:32 PDT
I will make new layout test case.
please let me know if you don't mean that.
Comment 6 zalan 2014-04-09 08:43:51 PDT
(In reply to comment #5)
> I will make new layout test case.
Thanks.
Comment 7 Miyoung Shin 2014-04-11 05:02:32 PDT
Created attachment 229125 [details]
Patch
Comment 8 Darin Adler 2014-04-12 12:43:03 PDT
Comment on attachment 229125 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=229125&action=review

> Source/WebCore/page/EventHandler.cpp:1325
> +    // Mouse pointer is possible to stay within the page even if hitTest's result is empty.
> +    // Cursor should be reset by the pointer type

I don’t understand this comment. Why is there no code change to go with it?
Comment 9 Miyoung Shin 2014-04-12 18:03:51 PDT
(In reply to comment #8)
> I don’t understand this comment. Why is there no code change to go with it?
Sorry, I've changed wrong code. I will reupload it soon.
Comment 10 Miyoung Shin 2014-04-12 18:07:49 PDT
Created attachment 229214 [details]
Patch
Comment 11 Build Bot 2014-04-12 19:08:41 PDT
Comment on attachment 229214 [details]
Patch

Attachment 229214 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4560572200779776

New failing tests:
fast/events/mouse-cursor-reset.html
Comment 12 Build Bot 2014-04-12 19:08:44 PDT
Created attachment 229216 [details]
Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-16  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 13 Miyoung Shin 2014-04-13 00:41:31 PDT
Created attachment 229222 [details]
Patch
Comment 14 Miyoung Shin 2014-05-25 18:13:01 PDT
darin, could you take a look?
Comment 15 Miyoung Shin 2014-07-15 08:02:19 PDT
Created attachment 234928 [details]
Patch
Comment 16 Miyoung Shin 2014-07-15 08:37:01 PDT
Created attachment 234930 [details]
Patch
Comment 17 Darin Adler 2014-07-20 08:57:43 PDT
Comment on attachment 234930 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=234930&action=review

Can’t review because the patch doesn’t explain itself sufficiently in comments or in change log.

> Source/WebCore/rendering/RenderLayer.cpp:4547
> +        if (!request.isChildFrameHitTest() && (request.active() || request.release() || (request.move() && renderer().frame().isMainFrame())) && isRootLayer()) {

I don’t understand how this change relates to the change log comment.

This says that the main frame is a special case, and that the “move” flag is relevant. Nothing in the change log helps explain why that’s correct to consider.

My guess is that for subframes we’d like to fall back to the main frame and that’s why the isMainFrame test is relevant, but I’m not sure that’s the reason. And I have no idea why checking the move() boolean is relevant.
Comment 18 Miyoung Shin 2014-07-21 01:34:02 PDT
I'm sorry to remain the insufficient description in the change log comment.
Let me give you a explain in more details for this patch.

This is a test page's layout.
|                                          |
|                ...                       |
|     HTML & Body's Rendering box          |
|__________________________________________|
|button|     x
|______|     ↑
             |___ there isn't any element here.

Currently, we can see the 'null' value when getting the result of hitTest for MouseMove event on x coordinates, and can't update mouse cursor like other browsers if mouse cursor moves the button coordinates to the x coordinates.
I believe that the visible expectation is to change a hand pointer of mouse cursor to a general pointer.

But I met with a difficulty like below.
- Should we cope with "null" value ?
- or, Do we guarantee that hitTest shouldn't return "null" value?

According your opinion, I tried to dig out second question.
I refereed the document.elementFromPoint's behaviour because we can see HTML Element value as a result of elementFromPoint under the same conditions.
I think you've already known as far as here.

I found that in case of elementFromPoint hitTestRequest is set by "Active" option, and the hitTest result updates to HTML Element again as the hitTest's result is "null".

So, I added "Move()" flag at there to ensure HTML Element as hitTest result for MouseMove event like elementFromPoint's active() option.

In case of isMainFrame, your guess is right.
I intended to do so because this change should have effect on only main frame.
If I remove isMainFrame condition, it will have a bad effect to mouseEnter & MouseLeave event's result. Anyway, I don't think we need this change for sub frame.

Thank you for your feedback and please let me know if I'm wrong.
Comment 19 Darin Adler 2014-07-21 17:12:00 PDT
(In reply to comment #18)
> I found that in case of elementFromPoint hitTestRequest is set by "Active" option, and the hitTest result updates to HTML Element again as the hitTest's result is "null".
> 
> So, I added "Move()" flag at there to ensure HTML Element as hitTest result for MouseMove event like elementFromPoint's active() option.

OK. This doesn’t just affect cursor adjustment. It affects mousemove DOM events too. So is that OK? Does that match other browsers’ behavior? Can we make a test case?
Comment 20 Miyoung Shin 2014-07-22 20:57:56 PDT
(In reply to comment #19)
> OK. This doesn’t just affect cursor adjustment. It affects mousemove DOM events too. So is that OK? Does that match other browsers’ behavior? Can we make a test case?

At first, I focused to check the mouse cursor's type and as I mentioned before, we works different with others.

When mouse cursor moves button coordinates and x coordinates,
FF & chrome & IE10 : it changes the cursor type from hand type to arrow type.
Safari : it doesn't change the cursor type without my patch.

But I realized that all browsers have a different layout result from document.elementFromPoint() when I made a test case for mousemove event.
- FF : for mainframe and subframe when mouse cursor is on x coordinates, HTML element is hit.
- IE10 : under the same condition, Body element is hit.
- Chrome : nothing. It seems that they are handling just the null value of hitTestResult.
- Safari : nothing, too.
           with this patch, for main frame, HTML element is hit.

Do I need to follow FF way?
If then, I need to change my patch to remove inMainFrame condition to work like FF unlike originally intention.
Of cause, I guess I need to check mouseenter/leave event as well because it will be affected by the hitTest's Result.
I'd like to seek your advice. Thanks.
Comment 21 Michael Catanzaro 2016-09-17 07:11:11 PDT
Comment on attachment 234930 [details]
Patch

Hi,

Apologies that your patch was not reviewed in a timely manner. Since it's now quite old, I am removing it from the review request queue. Please consider rebasing it on trunk and resubmitting.

To increase the chances of getting a review, consider using 'Tools/Scripts/webkit-patch upload --suggest-reviewers' to CC reviewers who might be interested in this bug.