-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Search.js in doxygen documentation module not working on mobile #196
Comments
Search works fine on iOS with both Safari and Firefox. This is likely an Android bug. |
I just tried this on a Samsung phone (Android, Chrome) and it works just fine as well. Autocomplete inserts text, but it is selected, meaning that as you type the selection (inserted text) gets replaced. It works the same way on all platforms and browsers I've tried so far, and is very convenient. Disabling this behavior would be a reduction in usability. |
Hm, strange, on my phone the added text is selected, but when I continue to type, the text is inserted before the added text instead of replacing it. I agree that finding a fix is better than removing the functionality. |
(Sorry for embarrassingly late replies, finally got a chance to get back to this project.) Hmm, seems like I can reproduce this, although maybe a bit differently(?):
So I suppose there's something weird in how I implement the autocompletion highlight, causing mobile browsers to not highlight in certain circumstances, which then results in the autocompleted text not being overwritten when typing? |
I tried doing the same as you @mosra So yeah, definitively something weird going on here, which is why I disabled this on mobile for my own documentation page |
Eh, stupid Chrome, of course it's the WONTIFX bug that's linked from the comment in the code (https://bugs.chromium.org/p/chromium/issues/detail?id=118639) or maybe some variant of it: Lines 734 to 752 in fb5fd85
All Trying to find another solution for this. |
Huh, but that's different from what I see here. I get |
I get double m's on both firefox and chrome on my mobile |
I suspect it's maybe also related to the virtual keyboard you use. Which one do you have? I'm on the vanilla gboard I think. |
I use gboard as well.. |
This patch should disable autocompletion completely on the buggy Android virtual keyboard, do you have an easy way to try it out? diff --git a/documentation/search.js b/documentation/search.js
index 5346fa63..ce7a7dea 100644
--- a/documentation/search.js
+++ b/documentation/search.js
@@ -739,15 +748,25 @@ if(typeof document !== 'undefined') {
excluding Ctrl-key, which is usually not for text input. In the
worst case the autocompletion won't be allowed ever, which is
much more acceptable behavior than having no ability to disable
- it and annoying the users. See also this WONTFIX Android bug:
- https://bugs.chromium.org/p/chromium/issues/detail?id=118639 */
- } else if(event.key != 'Backspace' && event.key != 'Delete' && !event.metaKey && (!event.ctrlKey || event.altKey)) {
+ it and annoying the users. */
+ } else if(event.key != 'Backspace' && event.key != 'Delete' && !event.metaKey && (!event.ctrlKey || event.altKey)
+ /* Don't ever attempt autocompletion with Android virtual
+ keyboards, as those report all `event.key`s as
+ `Unidentified` with `event.code` 229 and thus we have no way
+ to tell if a text is entered or deleted. See this WONTFIX
+ bug for details:
+ https://bugs.chromium.org/p/chromium/issues/detail?id=118639
+ Another option would be to use inputEvent there and catch
+ deletion. */
+ && event.key != 'Unidentified'
+ ) {
Search.autocompleteNextInputEvent = true;
/* Otherwise reset the flag, because when the user would press e.g.
the 'a' key and then e.g. ArrowRight (which doesn't trigger Meanwhile I'm trying to find a better solution that doesn't just kill the feature altogether. |
Okay, I think I have it -- for Chrome at least, if I delay-call
Thanks in advance for a confirmation! :) |
I tried your tmp url. Now I don't get the duplication issue on chrome, but backspace is still broken. On firefox both errors are still there |
I finally managed to reproduce both of your issues in Firefox. Will report if I get anywhere, thanks for the help so far 👍 |
What a mess.
I'm not sure why it has to delete the whole word, possibly it's some shitty workaround in order to trigger proper text reshaping? ugh |
I give up. The amount of suffering is just too much for such a minor feature. The version at https://tmp.magnum.graphics/search-android-nope-nothing/ should have autocompletion disabled for both Chrome and Firefox on Android. If you can confirm it's indeed the case (so |
Thanks for trying anyway. I proposed in this PR #197 to just disable autocomplete completely for mobile. Maybe that is a better option if this bug relates to the virtual keyboard on mobile devices and not the browsers? |
Thanks for the confirmation! I merged the patch as b598223, and stashed the unfinished attempt to fix this properly as #213. Since @crisluengo confirmed this works fine with iOS (and the Safari browser, no matter if skinned as FF or Chrome), I'd keep it working there. It's likely due to how Android implements virtual keyboard, which is reflected in the uselessness of key events. OTOH, if I connect a physical keyboard to Android or use the "remote Chrome" and enter keystrokes from there, there are no problems whatsoever. So I think the way I detect and skip autocompletion is neither too narrow nor too broad. Thanks for proposing #197 nevertheless. I have to admit the "mobile detection" monstrosity that lists every vendor / device name ever in existence is quite frightening 😆 How can one possibly be sure it won't have any false positives? Or am I supposed to refresh that regexp after every CES to accomodate for fresh batches of devices? 😅 |
Tested this on Android with both chrome and firefox.
The autocomplete inserts text while typing, making it impossible to search.
It seems to me that the code that marks the autocompleted text to be replaced when the next key is typed, is not working.
I can contribute to a fix, if anyone can point to where in the code the issue might be.
The text was updated successfully, but these errors were encountered: