Emoji selector crashes the message editor in Android / Chrome

bug
1

This issue has been around for couple of months, and I can reproduce it here at Meta using an Android/Chrome device (both latest) – actually this has been happening for several months, since January or so.

The message editor seems to “crash” every now and then when one chooses a proposed emoji.

Repro steps:

  1. Post a reply to a topic, add some text
  2. Start adding a thumbs up by typing “:+1” and then tap the proposed emoji :+1:

What happens:

The editor seems to somehow crash, or is redrawn. Some written text is typically lost.

It is not a 100% repro, but I can easily hit the issue within a minute of fiddling.

2 Likes
2

What exact version of Android/ Chrome are you using?

3

I can reproduce. Follows the stack trace:

_application-bfbda341c2eb6dd7d61c681e17bdccec057c30e045ddc332927a7363150e9b1b.js:16386 Uncaught TypeError: Cannot read property '0' of null
    at HTMLLIElement.<anonymous> (application-bfbda341c2eb6dd7d61c681e17bdccec057c30e045ddc332927a7363150e9b1b.br.js:1)
    at HTMLLIElement.dispatch (ember_jquery-36a23101c869ab0dc53fc908de69adb785731593573d32bdeef416acc1076ef4.br.js:1)
    at HTMLLIElement.d.handle (ember_jquery-36a23101c869ab0dc53fc908de69adb785731593573d32bdeef416acc1076ef4.br.js:1)
(anonymous) @ application-bfbda341c2eb6dd7d61c681e17bdccec057c30e045ddc332927a7363150e9b1b.br.js:1
dispatch @ ember_jquery-36a23101c869ab0dc53fc908de69adb785731593573d32bdeef416acc1076ef4.br.js:1
d.handle @ ember_jquery-36a23101c869ab0dc53fc908de69adb785731593573d32bdeef416acc1076ef4.br.js:1

That is this line

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/discourse/app/lib/autocomplete.js#L308

Error happens because selectedOption is 0 (single suggestion aka the first) while autocompleteOptions is somehow null.

Investigating why now…

So I’m not sure why so far. At first I was suspecting this PR from @Osama

https://github.com/discourse/discourse/pull/11637

But I added quite a few breakpoints and can’t really find “who” is mutating autocompleteOptions and setting it as null.

Having autocompleteOptions be from the scope of the parent closure from two levels up is also quite weird and makes the code a bit hard to follow up/debug.

8 Likes
4

Android 11/Chrome 89, the latest available.

1 Like
11

@ljpp / @falco can you still repro this bug? I’ve tried lots of variations of inserting an emoji and couldn’t repro not even once. If you can still repro, can you share the exact steps please? Thanks!

1 Like
12

Yes, it still crashes on Chrome Android for me. You must follow the instructions 100% for it to repro, as it’s very specific.

1 Like
15

Just tried and could not easily reproduce this. I am however quite certain that I have seen this over the summer a couple of times, accidentally when typing a real post.

Edit: I’ll ask our community if they could provide more extensive test coverage.

Edit 2: Oh yes, I reproed it myself now. Our community is now testing and looks like there are some more repros by regular users.

1 Like
16

Just reproed this accidentally, so this is still open.

18

We still have this on our list, we are just finding this fiendishly hard to reproduce and debug.

I can’t really put a #pr-welcome on this cause it is way too hard. We will revisit this at some point, putting a 6 months timer on it.

Is this issue still persisting?

19

Yes it is. Personally I could not reproduce it with 15 minutes of aggressive fiddling, so I asked our community for support. Several users were able to repro and one even proved it with a screenshot. Symptoms are unchanged - once you tap the thumb up -emoji on the selector, the keyboard folds away, and some written content is lost in the editor.

I used to repro this very easily, but now I can’t seem to nail it. So the issue is there, but it is not a 100% repro. We will keep on fiddling and trying to figure out if there is some UI-step that triggers this.

1 Like
20

Can you ask someone that can reproduce it, to see if they can do it on try.discourse.org

21

I just reproduced it without trying.

Firefox 94.1.1 (Build #2015842491)

And Chrome 95.0.4638.74

2 Likes
22

I feel like non-native platform emoji selectors are the real bug here. :wink: The trivial workaround is, use the native emoji picker in your OS of choice?

23

Sure there is a workaround usually. Not always, for instance there may be custom emoji in use on Discourse.

Either way, reloading the page and losing written content is poor behaviour we should fix.

24

Ok, I’ve looked into this again today and managed to repro after switching the virtual keyboard on my phone to Gboard. Gboard sometimes fires keydown and keyup events twice for a single key press and if that happens for the last key you press before selecting an emoji from autocomplete, you’ll get a crash.

I’m not sure what causes Gboard to fire these events twice, but it seems to depend on what you’ve typed and your Gboard settings.

The double events cause a crash due to the way our autocomplete library is designed. So the library listens to both keydown and keyup events, and on keydown it clears the autocomplete suggestions and on keyup it offers new suggestions based on the new autocomplete term.

However, there is a small guard/optimization to protect the library from doing duplicate work when the term has not changed since the previous suggestions, and this where the issue occurs. The first pair of keydown and keyup events clears the old suggestions and offers new ones as expected, but the second erroneous pair will clear the suggestions again on keydown but won’t offer new ones on keyup because the autocomplete term has not changed.

The only least hacky “fix” that I can think of is removing the guard/optimization so the library always offers new suggestions on keyup, but I’m not sure if this is desired or worth it.

I know that we want to rewrite our autocomplete library at some point (it’s some of the oldest code in the codebase and desperately needs a rewrite), so maybe this bug could wait until we get to the rewrite?

4 Likes
25

Is the crash a “infinite loop” of sorts? Can we simply track that we just hit a feedback loop and clear stuff up?

1 Like
26

No, the crash is accessing a null variable. When the suggestions are cleared the autocompleteOptions variable is set to null and then when you tap on one of rendered suggestions we expect autocompleteOptions to be an array but it’s null.

This gives me another idea: perhaps we could check if autocompleteOptions is null when a suggestion is selected and if the variable is null then we re-fetch the suggestions for the given term?

2 Likes
27

Yeah this sounds like a fine workaround.

2 Likes
28

Ah, I feel so dumb for not reporting this detail. My previous phone was a clean Android One so it had the Gboard as default, and it has followed me ever since to the next device (Sammy A42 5G). But most Android devices sold today use a 3rd party keyboard app by default

Great to see this progress.

2 Likes
29

I’ve merged my fix for this bug:

The fix has been deployed to Meta and to your site @ljpp, let me know if your users can still repro.

5 Likes