Summary: | Caret stops blinking after context menu shown | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jonathan Hammer <jonathan> | ||||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Minor | CC: | commit-queue, dino, jonathan, megan_gardner, rniwa, thorton, webkit-bug-importer, wenson_hsieh | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Local Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=40533 | ||||||||
Attachments: |
|
Description
Jonathan Hammer
2018-11-15 14:24:58 PST
Yes, you should be able to send eventSender.mouseDown(2) or eventSender.keyDown('menu') to trigger a context menu. It may only work in WebKitLegacy / WebKit1 though... so you might need to run the test with -1. (In reply to Ryosuke Niwa from comment #1) > Yes, you should be able to send eventSender.mouseDown(2) or > eventSender.keyDown('menu') to trigger a context menu. It may only work in > WebKitLegacy / WebKit1 though... so you might need to run the test with -1. Sorry I should have clarified: my question is how would the test be able to determine if the caret was blinking or not (after the menu is dismissed)? Oh, you probably have to add some method to internals to expose whether FrameSelection::m_caretBlinkTimer is active or not. (In reply to Ryosuke Niwa from comment #3) > Oh, you probably have to add some method to internals to expose whether > FrameSelection::m_caretBlinkTimer is active or not. Ah, got it. Didn’t know about internals until just now. I’ll look into it. Thanks! Created attachment 355081 [details]
Patch
Proposed patch.
Comment on attachment 355081 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355081&action=review Thanks for the fix! The code change looks good but there are a few nits to fix. > Source/WebCore/page/EventHandler.cpp:2883 > + Nit: Whitespace. > LayoutTests/fast/events/contextmenu-dismiss-blink-caret.html:3 > + <div id="contenteditable" contenteditable>This tests whether the caret continues to blink after the context menu is dismissed.</div> Please add an instruction to run this test manually in the browser. This is important so that QA folks, etc... can run this test manually to verify the fix, and also in the future this test ever gets broken, to be able to bisect using a browser. Created attachment 355100 [details]
Patch
Revised patch attached
Comment on attachment 355100 [details]
Patch
Let's land this. In the future, please also set cq? so that someone who is a committer can cq+ the patch to land it.
Comment on attachment 355100 [details] Patch Clearing flags on attachment: 355100 Committed r238345: <https://trac.webkit.org/changeset/238345> All reviewed patches have been landed. Closing bug. |