Skip to content
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

Unexpected delete behavior in android #2702

Closed
hecor opened this issue Apr 23, 2019 · 18 comments
Closed

Unexpected delete behavior in android #2702

hecor opened this issue Apr 23, 2019 · 18 comments

Comments

@hecor
Copy link

hecor commented Apr 23, 2019

Do you want to request a feature or report a bug?

Bug.

What's the current behavior?

android-slate-bug

What's the expected behavior?

Delete a character each time.

system

Vivo phone, android 8.1

@thesunny
Copy link
Collaborator

@hecor Thanks for the screencast.

Just to clarify what you are doing here. You go to the end of a line and hit enter. Insert a word either through IME or regular means. Hitting enter deletes the entire word instead of one character.

I'll take a look after I migrate the Android plugins to use some new constructs I'm working on. I think I know what's happening though.

@hecor
Copy link
Author

hecor commented Apr 24, 2019

@thesunny Thanks for the quick reply.
Actually, I don't need go to the end of a line, everything I input will be deleted one time after I press delete. It seems what I input doesn't belong to the whole text. When I save the content in my product, what I input will be disappeared.

@thesunny
Copy link
Collaborator

@hecor I'm not sure what's going on exactly until I do some more analysis but my guess is that the delete in Android is doing something nasty and so we take a DOM snapshot before doing the delete. However, it's probably using the wrong snapshot, one that was taken before the text was entered.

I'll look out for this when I get back to that part of the code.

@hecor
Copy link
Author

hecor commented Apr 24, 2019

OK, hope to have your good news soon~

@AnatolyShirykalov
Copy link

I also had a crush typing backspace in your demo page
ezgif com-video-to-gif
Asus ZB602KL. Android 8.1.0, Gboard, Chrome 73

@thesunny
Copy link
Collaborator

thesunny commented May 1, 2019

I think this bug is fixed now. You can test it out here: https://thesunny.github.io/slate/#/rich-text

Edit: Fixed but not merged into master FYI

@thesunny
Copy link
Collaborator

thesunny commented May 1, 2019

I'm working on fixing the remaining Android bugs now.

In order to be more efficient, please submit bugs and help with testing over in this issue:

Fix all showstopper bugs in Slate on Android (Help us by reporting and testing bugs)
#2726

@glebtv
Copy link

glebtv commented May 1, 2019

For me it's worse with this fixes:

  1. Deleting selection is broken, it reappears:
    del-sel

  2. Deleted content reappears after releasing backspace:
    reappear

  3. Crash if pressing a letter right after bacspacing:
    backspace than letter

Oneplus 5t, android 9, gboard

Same problems + crash in the end on mediapad m5, android 8:

m5

(i'm NOT saying this was much better for me before this fix)

@thesunny
Copy link
Collaborator

thesunny commented May 1, 2019

Can you tell me if it works on your browser if you release backspace before the start of block? I haven’t tested arbitrarily long deletes through blocks and I can see why it would fail but I want to make sure the non wrapping use case is fixed. It works for me.

It can be difficult to tell if it’s a bug because it’s a different browser environment or a different action so I’d like to make sure that you see my in line backspace behavior.

The select delete issue I haven’t tested. That’s a separate bug. That one might be hard.

@hecor
Copy link
Author

hecor commented May 1, 2019

Hi @thesunny It didn't fix my problem too, actually my another Huawei android phone which is fine before have the same problem now.

@thesunny
Copy link
Collaborator

thesunny commented May 1, 2019

@hecor can you clarify that you can or cannot hold down backspace and release before you cross the start of the line and have it work? Try these tests I made here:

https://thesunny.github.io/slate/#/composition/delete

If you can't do that, this might be an issue that doesn't present on the simulator but does present on hardware which will be painful to work on.

Also, please let me know Android API version and device names and which ones worked before and don't work now. By the way, I haven't incorporated any Android 7 fixes into this version of the code yet so those will not work at all.

Note: The select then delete is a separate issue. As is the issue of holding backspace through the start of line.

Also, could you let me know if hitting delete one at a time causes issues?

@thesunny
Copy link
Collaborator

thesunny commented May 1, 2019

@hecor @AnatolyShirykalov @glebtv

Can you let me know how you made the screen recordings? I would like to add the tool to the bug reporting post I made.

@Nantris
Copy link
Contributor

Nantris commented May 1, 2019

@thesunny, I got you: #2726

@thesunny
Copy link
Collaborator

thesunny commented May 1, 2019

Although this thread seems to be talking about one issue, it is actually several. When talking about an issue, please refer to it's number below. If I believe the issue is fixed, I will check it:

  • 1 Android 8&9 - Go to end of line. Hold down backspace but not past the beginning of line.
  • 2 Android 8&9 - Go to middle of line. Hold down backspace but not past the beginning of line.
  • 3 Android 8&9 - Go to end of line. Delete one character at a time. When you are right before the space and you hit enter again, the word will reappear when it shouldn't.
  • 4 @hecor Android 8.1 - Go to the end of a line. Hit backspace one at a time until you delete to the end of the previous word. Hit enter. Type some words. Hit backspace. It should delete only one character but the entire word is disappearing. JUST FIXED.
  • 5 @AnatolyShirykalov Android 8.1 - Hold down backspace from end of line past the beginning of the line. The crash shouldn't happen at the beginning of the line but funky behavior if you delete past 2 lines.
  • 6 @glebtv Android 9 - Selecting text and deleting it is broken
  • 7 @glebtv Android 9 - Deleting with holding down from end of "This is editable rich text" line to beginning of line. After releasing backspace the content re-appears.
  • 8 @glebtv Android 9 - Deleting from end of second line to middle of first line then hitting a key causes a crash. I can't replicate this but please let me know if this issue still exists.
  • 9 @glebtv Android 9 - Entering then deleting multiple lines causes crash
  • 10 Android 8&9 - Holding down backspace for more than two blocks of text will put DOM in bad state. Doing anything after will likely result in a React error.

@glebtv thank you so much for the bug posts but I am having trouble reliably recreating these issues. I'm not convinced this is a difference from the simulator yet (although it could be). A big help would be to be very specific about the actions starting with the pre-existing content. There appears to be some content missing from the start in the scenarios.

In other words, starting from fresh content. What exactly did you type. How did you type it? (gesture, one key at a time) How did you delete it? One backspace at a time or hold down the delete? In Android, unlike the other browsers, you don't handle delete one way. It differs how you hold the key, everything you did before it, what's the state of the Slate DOM, did you start in a word, at the end of word, at the end of line, etc. I can determine some of it from the screencast but I cant get all of it. Also, you may be able to glean some information by looking at the Slate value which is at the bottom of the composition rich examples which I'm publishing now.

https://thesunny.github.io/slate/#/composition/rich

@Nantris
Copy link
Contributor

Nantris commented May 1, 2019

I think I've re-reported a few mentioned here. Here's my best guesses as to mapping issues here to my new reports.

Anything left on this list in a few days I'll try to figure out how to re-create, if they still can be.

@thesunny
Copy link
Collaborator

thesunny commented May 2, 2019

Hold everything. The fix to the range deletions broke the regular deletions.

@glebtv
Copy link

glebtv commented May 3, 2019

It is very easily reproducuble for me, just deleted some text by holding backspace, allowing delete to wrap over multiple paragraphs.

This is hardware, not emulator.

I have some keyboard settings configured, like autocorrect and suggestions disabled on gboard.

Also, slate value in composition demo doesnt change for me in most of editing scenarios, although some work.

Here is an updated video with slate value included, from oneplus 5t, including crash at the end.

https://www.dropbox.com/s/1stgqocg8joi4td/2019_05_03_09_39_32.mp4?dl=0 - adding as a real video so you can pause/rewind. When the key I press is not visible, it is backspace. (gboard does not highlight it on press somewhy)

Crash is hard to reproduce for me (1 crash per 3-5 minutes), but incorrect update of slate value and reappear of deleted content are easy / all the time.

@ianstormtaylor
Copy link
Owner

Hey, there are lots of open Android-related issues that are out of date with the latest 0.50 release because the codebase was completely rearchitected. We'll need to figure out how to have proper support for Android going forward based on beforeinput events somehow. I'm tracking this in #3112, so I'm going to close this in favor of that one.

If it cannot be implemented simply in core with beforeinput we'll likely need to have a separate plugin for android support be created, because it's too much of a departure from all of the other simpler logic in core and very hard for me to test or respond to issues. Using a separate plugin should be possible now as the newest version simplifies a lot of the internals.

Thank you for understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants