WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
NEW
131172
Mouse Cursor doesn't change appropriately.
https://bugs.webkit.org/show_bug.cgi?id=131172
Summary
Mouse Cursor doesn't change appropriately.
Nathan Hammond
Reported
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.
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
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Miyoung Shin
Comment 1
2014-04-06 02:57:36 PDT
Is there anyone to follow this issue ? If not, can I pick this one ?
Miyoung Shin
Comment 2
2014-04-07 05:36:08 PDT
Created
attachment 228728
[details]
Patch
zalan
Comment 3
2014-04-08 07:42:55 PDT
Please add a test case.
Miyoung Shin
Comment 4
2014-04-08 08:08:13 PDT
do you mean new layout test case for this issue?
Miyoung Shin
Comment 5
2014-04-09 08:42:32 PDT
I will make new layout test case. please let me know if you don't mean that.
zalan
Comment 6
2014-04-09 08:43:51 PDT
(In reply to
comment #5
)
> I will make new layout test case.
Thanks.
Miyoung Shin
Comment 7
2014-04-11 05:02:32 PDT
Created
attachment 229125
[details]
Patch
Darin Adler
Comment 8
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?
Miyoung Shin
Comment 9
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.
Miyoung Shin
Comment 10
2014-04-12 18:07:49 PDT
Created
attachment 229214
[details]
Patch
Build Bot
Comment 11
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
Build Bot
Comment 12
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
Miyoung Shin
Comment 13
2014-04-13 00:41:31 PDT
Created
attachment 229222
[details]
Patch
Miyoung Shin
Comment 14
2014-05-25 18:13:01 PDT
darin, could you take a look?
Miyoung Shin
Comment 15
2014-07-15 08:02:19 PDT
Created
attachment 234928
[details]
Patch
Miyoung Shin
Comment 16
2014-07-15 08:37:01 PDT
Created
attachment 234930
[details]
Patch
Darin Adler
Comment 17
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.
Miyoung Shin
Comment 18
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.
Darin Adler
Comment 19
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?
Miyoung Shin
Comment 20
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.
Michael Catanzaro
Comment 21
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.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug