-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
Safari's Selection is broken in shadow roots #414
Comments
I can't reproduce this—typing or copying does the right thing when I have a selection in Mobile Safari. Select all is available in the menu when I tap the cursor, and seems to work. |
Don't know why my mention isn't being shown here... Please have a look at the gif I uploaded in the issue mentioned above. |
I'm getting the same results when using the Codemirror 6 demo (https://codemirror.net/6/) but its failing as part of the recent Codemirror 6 implementation within Home-Assistant. Seeing the same issues mentioned. |
I created a repo, it happens as soon as I use Codemirror in a The repo can be found here: (it also shows a bug in the YAML language with the |
I am not a JS dev so please forgive if this is irrelevant. I hope these details can add hints when you open the edit window on e.g. an iPad and tap to place the cursor at the end of a line and hit the backspace (only way to delete text on IOS) nothing happend. If you start typing the characters are added at the beginning of the first line of text in the window. So what is happening with backspace is probably that the code thinks you are at position 0 at line 0 and behaves like that. I would assume a lost track of cursor would also create havoc when you copy and paste as the other reports describe It seems the bug relates to the actual cursor position being lost. Only on IOS. No issue on Linux or Windows browsers |
@marijnh would you have any pointers where I could look for this? |
Not really—it's probably some Mobile Safari misbehavior that I'm not aware of. I guess reproducing the issue and trying to trace the source of the problem is the only reasonable way to diagnose this. |
One of the few looking at the code 😅
Is this enough for that? #414 (comment) I tried to look at the code, but not sure where to start... If there is anything I can do to help, let me know. |
One thing I do notice is that https://github.com/GoogleChromeLabs/shadow-selection-polyfill |
OK, yeah so Safari on the desktop uses If I remove that return, it kinda starts working (I can type again!), but not fully, selections are still wrong (there is a reason it was ignored probably!)... |
(Touch events are ignored because our code doesn't implement proper touch selection — handle dragging, and whatever custom behavior the various platforms have.) |
Okay, so on closer look this is in fact something I've encountered with ProseMirror before. Safari still doesn't implement any reasonable way to get the selection inside a shadow root. As you found, it'll just say the selection is on So yeah, this is bad. Apple apparently doesn't consider this a priority, and I'm not sure how we can work around it. Some research into whether any other contentEditable libraries are managing to work around this might be useful. |
Hm. It worked before the last HA release, so either a change in HA or a change in the codemirror version is responsible, that it is now not working anymore. So imho there is no need to search for other libraries, because the last working version is not that far away, ;) Before, editing, deleting, putting the cursor, etc. was working. Only the copy and paste from multiple line/block was not possible, and it copied only one line from the selection. |
@emufan That obviously also was not the right solution as you already mention, and one of the main targets for CodeMirror 6. In case you did not know, the change you are talking about is a complete rewrite of CodeMirror. |
This might be a solution... https://jsfiddle.net/dbalcomb/oLnkyv78/ ? Another option if you don't want to ship a polyfill with CodeMirror, is to let the user supply a All these polyfills are based on bugs/hacks or deprecated functions, the odds of it breaking again are quite high... |
No, was not aware of this. Sorry for that.
Depending on the line above, yes or no. Thought that were no major changes and marijnh was only not asware, that it was working right before. So in this case, I thought that it is easier to search in the last working release instead of searching other libraries. And then it was imho a useful comment. |
@bramkragten I saw that polyfill before too, but it doesn't look very confidence-inducing or complete. Allowing client code to pass an implementation would not really be any better than just having them polyfill There's apparently a Webkit bug for this, but it doesn't seem to be taken very seriously https://bugs.webkit.org/show_bug.cgi?id=163921 |
Interestingly (and ridiculously), setting a selection inside a shadow root through the top-level Selection object does work. So the problem is just with reading the selection. |
It would allow for ponyfills instead of just polyfills, but other than that no... |
FIX: Add a workaround for Safari's broken selection reporting when the editor is in a shadow DOM tree. Issue codemirror/dev#414
I've pushed a patch. It's not great, but it's also not entirely terrible, and should limit any potential dodginess it introduces to the shadow dom + safari situation. I only tested on desktop (Epiphany) so far, but the general problem could also be observed there in some corner cases. Please take a look and let me know how it works for you. |
Thanks for the patch, it is not quite there yet though. The selection seems to work, I can copy and paste, so does typing on a new line, but when typing at the end of an existing line, my cursor goes backward while typing instead of forwards: RPReplay_Final1619427822.mp4 |
Maybe a bit clearer here: So typing the first pieces RPReplay_Final1619428496.mp4 |
I'm not seeing those failures on an iPhone w/ iOS 14.4.2 |
Mmmm I updated my Glitch and also don't have this behavior there. Will do some more digging. |
Got a repo here: https://shrub-smoggy-phosphorus.glitch.me/ It is not just iOS btw, it is also Safari on Mac. When using |
@marijnh could you create a new release with this patch so I can get it into our beta and get more feedback? (hopefully just positive this time!) |
Sure, I've tagged 0.18.11 |
Thanks for the fast fix. I’m not able to reproduce this issue with the latest codemirror version in home assistant beta 5 anymore. (Tested with iOS 14.5) |
Oh no, I found one more issue. If you use the cursor on the touchpad and scroll over the Edge of the Screen, and it scrolls in a Textfield the cursor gets lost and you can't type text unless you tap somewhere to type. If you use the touchpad and to mark text above the Edge of the screen everything is working fine. |
Could it be that that same thing also happens without shadow DOM? If so—could you open a separate issue for it? |
It seems like this issue is back 😢 |
Yup, definitely came back. Feels like it‘s being worse even. No way to e.g. insert a new line or edit. RETURN or BACK makes cursor hop instantly to pos 1,1 in the code. |
Are the last two comments talking about Mobile Safari or Desktop? Did you verify that it is related to shadow roots? Does @codemirror/view 0.19.14 help? |
Affects Mobile Safari on iOS |
I'm having this issue via Home Assistant's iOS app. (home-assistant/iOS#1904) |
Yeah, that video pretty much describes the whole pain! |
Because that somehow makes beforeinput events on iOS asynchronous, breaking our shadow-root-getSelection kludge. FIX: Fix selection reading inside a shadow root on iOS. Issue codemirror/dev#414
So, it turned out that codemirror/view@e3cdb43 broke this, because (hold on to your hat), setting Attached patch seems to help for me—please test if you have a chance. |
@emufan said in Apr that this worked prior to Mar HA update:
I want to point out that HA users also began experiencing another editor issue immediately following a Feb 21 update to Home Assistant Frontend that updated to use CodeMirror 6. @marijnh could this somehow be obliquely related to the issue we discussed HERE? |
Possibly. So did you test the patch? |
I just tested the editor in the latest release of HA on iOS and it still exhibits the "jump to fist line" issue. That is my only exposure to Codemirror and I do not know if this patch has been included. Sorry. |
@marijnh, tested the patch and seems to work great! 🎉 Let's hope Apple gives this some love in the future 😞 |
Great, thanks for testing. I've released @codemirror/view 0.19.15 |
Wanted to chime in that I've ran into a similar issue with the new |
The Safari issue has been marked fixed a few months ago, but Safari 16.4.1 unfortunately still has the bug. |
Selections on iOS don't seem to work well, when selecting something it is shown correctly, but when you try to type, remove or copy the selection, it will either not work, or target some other line.
Also a select all doesn't work.
The explanation is a bit poor, hope you can reproduce it. I couldn't find a lead in the code...
The text was updated successfully, but these errors were encountered: