Hide mouse pointer while typing on Windows [chromium/src : main]
Ashish Kumar (Gerrit)
Message from Ashish Kumar![]( "Open in Gerrit")
Set Ready For Review
Related details
- Ashish Kumar
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rohan Raja (Gerrit)
Rohan Raja added 2 comments![]( "Open in Gerrit")
gfx::SingletonHwnd::GetInstance()->RegisterCallback(base::BindRepeating(Shouldn't we unregister the callback upon destruction ?
if (event.IsAltDown() || event.IsControlDown() || event.IsCommandDown()) {Isn't Ctrl+V action valid for hiding cursor? It can trigger editing actions, the cursor might obstruct pasted text content. Same for <Tab> key as well, it might lead to text editing in cases like textarea.
Related details
- Ashish Kumar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ashish Kumar (Gerrit)
Ashish Kumar added 2 comments![]( "Open in Gerrit")
gfx::SingletonHwnd::GetInstance()->RegisterCallback(base::BindRepeating(Shouldn't we unregister the callback upon destruction ?
I think no explicit unregister is needed. `base::CallbackListSubscription` is stored in the setting_change_subscription_ member and it's an RAII handle, its destructor removes the callback from the list, so it is automatically unregistered when the `CursorManager` is destroyed.
if (event.IsAltDown() || event.IsControlDown() || event.IsCommandDown()) {Isn't Ctrl+V action valid for hiding cursor? It can trigger editing actions, the cursor might obstruct pasted text content. Same for <Tab> key as well, it might lead to text editing in cases like textarea.
I have removed the TAB key check.
For CTRL+V, the trigger point here is typing not editing actions in general. So, Ctrl+V intentionally doesn't hide the cursor here. This aligns with the behavior with other platforms and also with native editing apps on Windows like notepad and MS Word.
Related details
- Rohan Raja
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitsuru Oshima (Gerrit)
Mitsuru Oshima added 1 comment![]( "Open in Gerrit")
return client && client->ShouldHideCursorOnTyping() &&I thought just checking TextInputClient is enough. (and this will disable
this behavior on native UI). Can you explain why you want to do this only in web contents?
Related details
- Ashish Kumar
- Dan Clark
- Rohan Raja
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ashish Kumar (Gerrit)
Ashish Kumar added 2 comments![]( "Open in Gerrit")
gfx::SingletonHwnd::GetInstance()->RegisterCallback(base::BindRepeating(Ashish KumarShouldn't we unregister the callback upon destruction ?
I think no explicit unregister is needed. `base::CallbackListSubscription` is stored in the setting_change_subscription_ member and it's an RAII handle, its destructor removes the callback from the list, so it is automatically unregistered when the `CursorManager` is destroyed.
Resolving.
return client && client->ShouldHideCursorOnTyping() &&I thought just checking TextInputClient is enough. (and this will disable
this behavior on native UI). Can you explain why you want to do this only in web contents?
The client based opt in/out control can help in cases for some browser views where hiding does not make much sense (e.g. The find bar view).
This CL is intentionally scoped to web contents; the browser views will be handled in a follow-up once I've gone through them in more detail to decide which should opt in.
Related details
- Dan Clark
- Mitsuru Oshima
- Rohan Raja
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitsuru Oshima (Gerrit)
Mitsuru Oshima added 1 comment![]( "Open in Gerrit")
return client && client->ShouldHideCursorOnTyping() &&Ashish KumarI thought just checking TextInputClient is enough. (and this will disable
this behavior on native UI). Can you explain why you want to do this only in web contents?
The client based opt in/out control can help in cases for some browser views where hiding does not make much sense (e.g. The find bar view).
This CL is intentionally scoped to web contents; the browser views will be handled in a follow-up once I've gone through them in more detail to decide which should opt in.
>The find bar view)
Can you elaborate why it doesn't make much sense? (e.g. CrOS does this on all text fields, including omnibox). What if UI implementation is changed to use WebUI?
Related details
- Ashish Kumar
- Dan Clark
- Rohan Raja
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ashish Kumar (Gerrit)
Ashish Kumar added 1 comment![]( "Open in Gerrit")
return client && client->ShouldHideCursorOnTyping() &&Ashish KumarI thought just checking TextInputClient is enough. (and this will disable
this behavior on native UI). Can you explain why you want to do this only in web contents?
Mitsuru OshimaThe client based opt in/out control can help in cases for some browser views where hiding does not make much sense (e.g. The find bar view).
This CL is intentionally scoped to web contents; the browser views will be handled in a follow-up once I've gone through them in more detail to decide which should opt in.
>The find bar view)
Can you elaborate why it doesn't make much sense? (e.g. CrOS does this on all text fields, including omnibox). What if UI implementation is changed to use WebUI?
I think the users usually click next/previous between queries while typing in the find bar. I noticed the same behavior of not hiding the pointer in the Notepad native app and Firefox also. That said, even hiding in the find bar would be workable since the cursor reappears on mouse move, so I don't have a strong opinion either way and a WebUI-based view would inherit this hiding behavior automatically.
Since I'm not fully aware of all the browser views, I wanted to proceed in two steps. But if you'd prefer default hide-everywhere, I'm happy to make the changes. Please let me know your thoughts.
Related details
- Dan Clark
- Mitsuru Oshima
- Rohan Raja
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitsuru Oshima (Gerrit)
Mitsuru Oshima added 1 comment![]( "Open in Gerrit")
return client && client->ShouldHideCursorOnTyping() &&Ashish KumarI thought just checking TextInputClient is enough. (and this will disable
this behavior on native UI). Can you explain why you want to do this only in web contents?
Mitsuru OshimaThe client based opt in/out control can help in cases for some browser views where hiding does not make much sense (e.g. The find bar view).
This CL is intentionally scoped to web contents; the browser views will be handled in a follow-up once I've gone through them in more detail to decide which should opt in.
Ashish Kumar>The find bar view)
Can you elaborate why it doesn't make much sense? (e.g. CrOS does this on all text fields, including omnibox). What if UI implementation is changed to use WebUI?
I think the users usually click next/previous between queries while typing in the find bar. I noticed the same behavior of not hiding the pointer in the Notepad native app and Firefox also. That said, even hiding in the find bar would be workable since the cursor reappears on mouse move, so I don't have a strong opinion either way and a WebUI-based view would inherit this hiding behavior automatically.
Since I'm not fully aware of all the browser views, I wanted to proceed in two steps. But if you'd prefer default hide-everywhere, I'm happy to make the changes. Please let me know your thoughts.
Editing does various things including select all and replace, cut&paste then edit, etc, so my recommendation is simply hide it.
I'm ok If you want to do it in a separate CL, although in that case, you'll end up removing the utility method you just added, so my recommendation is still just do it for all fields. (plus, there will be no inconsistency between webui and native. (e.g. tab search, side panels are webui)
Related details
- Ashish Kumar
- Dan Clark
- Rohan Raja
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ashish Kumar (Gerrit)
Ashish Kumar added 1 comment![]( "Open in Gerrit")
return client && client->ShouldHideCursorOnTyping() &&Ashish KumarI thought just checking TextInputClient is enough. (and this will disable
this behavior on native UI). Can you explain why you want to do this only in web contents?
Mitsuru OshimaThe client based opt in/out control can help in cases for some browser views where hiding does not make much sense (e.g. The find bar view).
This CL is intentionally scoped to web contents; the browser views will be handled in a follow-up once I've gone through them in more detail to decide which should opt in.
Ashish Kumar>The find bar view)
Can you elaborate why it doesn't make much sense? (e.g. CrOS does this on all text fields, including omnibox). What if UI implementation is changed to use WebUI?
Mitsuru OshimaI think the users usually click next/previous between queries while typing in the find bar. I noticed the same behavior of not hiding the pointer in the Notepad native app and Firefox also. That said, even hiding in the find bar would be workable since the cursor reappears on mouse move, so I don't have a strong opinion either way and a WebUI-based view would inherit this hiding behavior automatically.
Since I'm not fully aware of all the browser views, I wanted to proceed in two steps. But if you'd prefer default hide-everywhere, I'm happy to make the changes. Please let me know your thoughts.
Editing does various things including select all and replace, cut&paste then edit, etc, so my recommendation is simply hide it.
I'm ok If you want to do it in a separate CL, although in that case, you'll end up removing the utility method you just added, so my recommendation is still just do it for all fields. (plus, there will be no inconsistency between webui and native. (e.g. tab search, side panels are webui)
- Dan Clark
- Mitsuru Oshima
- Rohan Raja
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |