[pointer-lock] Cursor should become visible when exiting pointer-lock via ESC key.
Created attachment 296146 [details] Patch
Comment on attachment 296146 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296146&action=review > Source/WebCore/page/PointerLockController.h:50 > + enum ForceCursorVisibleFlag { > + DoNotForceCursorVisible, > + ForceCursorVisible, > + }; I suggest using an enum class: enum class ShouldForceCursorVisible { Yes, No }; I also suggest defining this on a single line instead of four lines. > Source/WebCore/page/PointerLockController.h:52 > void elementRemoved(Element*); These functions that take pointers are so old fashioned. They should take references unless there is some reason they need to work with null.
Comment on attachment 296146 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=296146&action=review > Source/WebCore/page/PointerLockController.h:51 > + void requestPointerUnlock(ForceCursorVisibleFlag); I suggest two different functions instead of one function with a flag. We can use the flag internally, but it is less clear for callers than two separate functions with good names would be.
Created attachment 296161 [details] Patch for landing
rdar://problem/29081718
Created attachment 296201 [details] Patch for landing
Comment on attachment 296201 [details] Patch for landing Rejecting attachment 296201 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: x86_64/RenderButton.dia -c /Volumes/Data/EWS/WebKit/Source/WebCore/rendering/RenderButton.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/RenderButton.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/PointerLockController.o page/PointerLockController.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output: http://webkit-queues.webkit.org/results/2629953
Committed r209394: <http://trac.webkit.org/changeset/209394>
Comment on attachment 296201 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=296201&action=review > Source/WebCore/page/PointerLockController.cpp:157 > + if (m_forceCursorVisibleUponUnlock) > + m_page.chrome().client().setCursorHiddenUntilMouseMoves(false); Shouldn't we set m_forceCursorVisibleUponUnlock back to false here?
Comment on attachment 296201 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=296201&action=review >> Source/WebCore/page/PointerLockController.cpp:157 >> + m_page.chrome().client().setCursorHiddenUntilMouseMoves(false); > > Shouldn't we set m_forceCursorVisibleUponUnlock back to false here? Gah, you're right. I'll fix that s in a follow-up.
Reopening to attach new patch.
Created attachment 296392 [details] Follow-Up Patch
Comment on attachment 296392 [details] Follow-Up Patch Clearing flags on attachment: 296392 Committed r209464: <http://trac.webkit.org/changeset/209464>
All reviewed patches have been landed. Closing bug.