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

GestureHandlerRootView interferes with text selection inside webview during Android back gesture #3196

Closed
Nantris opened this issue Oct 31, 2024 · 26 comments · Fixed by #3244
Assignees
Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snack or repo is provided

Comments

@Nantris
Copy link

Nantris commented Oct 31, 2024

Description

When doing a back gesture in Android - the GestureHandlerRootView causes bugs interacting with the underlying Webview and they do not seem to be avoidable and are without any workaround. This makes things feel beyond janky.

Steps to reproduce

  1. Use provided repro Expo Snack
  2. Do back gestures over the text, but don't release the gesture. You'll notice text is selected or the Android text zoom bubble appears (and may persist)
  3. Comment out the BuggyApp and enable the alternative version WorkingApp to see the intended behavior (text selection may change still, but will not set expanded text selections or cause text zoom bubble to appear)

Snack or a link to a repository

Snack repro

Gesture Handler version

2.20.2

React Native version

0.72.x through 0.76.x

Platforms

Android

JavaScript runtime

Hermes

Workflow

Expo managed workflow

Architecture

Paper (Old Architecture)

Build type

None

Device

Real device

Device model

Galaxy S24 (Android 14) -- Reproducible on all Android devices using gestures

Acknowledgements

Yes

@Nantris
Copy link
Author

Nantris commented Oct 31, 2024

You would think if you're clever enough you could work around this, but it does not appear to be the case. Even if you can detect this occurring, the Webview will still respond to the touch event if you set pointerEvents: 'none' on it, or inside of it.

@Nantris
Copy link
Author

Nantris commented Nov 1, 2024

If Android would let you persist the keyboard, it would be hard to work around this - but without that capability it's impossible to do anything that feels much less janky than the bug itself does.

@Nantris
Copy link
Author

Nantris commented Nov 2, 2024

I know it's been like 18 hours, but early bump because I've spent probably 60 hours on this issue.

@m-bert
Copy link
Contributor

m-bert commented Nov 4, 2024

Hi @Nantris! I've just tested this on the following devices:

  1. Pixel 6a emulator - in both cases text zoom bubble does not appear
  2. Samsung Galaxy S24 Ultra - app behaves exactly the same, no matter which component I use (BuggyApp/WorkingApp)

@m-bert m-bert added the Can't repro We can't reproduce this issue label Nov 4, 2024
@Nantris
Copy link
Author

Nantris commented Nov 4, 2024

Thanks for your reply @m-bert! I'll provide a video of exactly what I mean as soon as I can. I can produce this on all devices that use the gesture navigation (most recently tested on Galaxy S24.)

@Nantris
Copy link
Author

Nantris commented Nov 5, 2024

Apologies that I couldn't capture a screen recording sooner. Hopefully this will clarify the problematic behavior @m-bert. Apologies also for the oddly dark videos.

Working normally without GestureHandlerRootView

Works.smoothly.without.GestureHandlerRootView.mp4

Text zoom bubble gets stuck and expanded text selections can occur with GestureHandlerRootView

Buggy.with.GestureHandlerRootView.mp4

@m-bert
Copy link
Contributor

m-bert commented Nov 7, 2024

Which version of React Native do you use?

@Nantris
Copy link
Author

Nantris commented Nov 7, 2024

Thanks for your reply @m-bert. It's 0.76.2 I believe (off the top of my head) but reproducible at least as far back as 0.72.x, and probably back forever based on my recollection.

I think the Expo Snack uses 0.74.x since SDK 51 is the latest.

@Nantris
Copy link
Author

Nantris commented Nov 7, 2024

To clarify, the version we use is 0.76.1 - but it affects all those versions for sure.

@m-bert
Copy link
Contributor

m-bert commented Nov 8, 2024

I've just tested that on Pixel 6 (physical device) and I got exactly what on emulator - no zoom bubble.

@Nantris
Copy link
Author

Nantris commented Nov 8, 2024

Thanks for continuing to look into this @m-bert.

The manifestations can be a bit different depending on the exact gesture and underlying content, but it should be reproducible across devices.

Possible outcomes are:

  • Expanded text selection occurs
  • Zoom bubble becomes stuck up
  • Phantom vibrations (probably related to text selections changing)

I accidentally left the repro in a state where the bug would not be reproduced which could be to blame (I commented out BuggyApp)

This is a screen recording from my Pixel 4a running Android 13:

screen-20241108-134634.mp4

I'll also upload a screen recording from an emulator when I'm back at my PC.

Thank you so much again for taking a look at this!

@Nantris
Copy link
Author

Nantris commented Nov 8, 2024

On a Pixel 8 emulator running Android 14:

The text zoom bubble outcome seems less likely here, but the expanded text selections occur.

Android-back-gesture-interference.mp4

@Nantris
Copy link
Author

Nantris commented Nov 11, 2024

Thanks again for your replies @m-bert!

I wonder if you've had a chance to take a look at the additional videos and I wonder if the failure to reproduce on your end might be due to subtle differences in the gestures we're doing. When I produce the issue, the user begins a back gesture (let's say possibly by accident) - holds it for a moment - and then swipes back towards the edge to cancel it.

Is that already what you're doing? If you're not able to reproduce it, I wonder if you could share a screen recording and maybe I could identify a discrepancy between our two methods that leads to it not reproducing on your end.

Thanks for the great work and again for taking the time to take a look at this issue in the first place.

@Nantris
Copy link
Author

Nantris commented Nov 13, 2024

Is there anything I can do to help move this forward? It's presenting real usability issues for us and unfortunately it's entirely impossible to work around from our side - especially since react-navigation relies heavily on react-native-gesture-handler.

@m-bert
Copy link
Contributor

m-bert commented Nov 14, 2024

Hi @Nantris! I've been really busy this week so I didn't have a chance to look at it. I'll try to get back to it today and take a video of my emulator.

@m-bert
Copy link
Contributor

m-bert commented Nov 14, 2024

Okay, I've tried it on another emulator and I've finally managed to reproduce it. I'll look into it and get back to you when I find something 😅

@Nantris
Copy link
Author

Nantris commented Nov 14, 2024

Thanks so much for taking a look into this @m-bert; I really appreciate it! Please let me know if I can be of assistance in any way!

@m-bert
Copy link
Contributor

m-bert commented Nov 15, 2024

Hi @Nantris! To keep you updated, I've managed to get rid of the text selection and other problems. However, it is a tricky way and I'm not yet sure if we are supposed to do this. We will talk about it on Monday meeting 😅

@m-bert m-bert removed the Can't repro We can't reproduce this issue label Nov 15, 2024
@Nantris
Copy link
Author

Nantris commented Nov 15, 2024

Thank you so much again for looking into this @m-bert! It's exciting to hear about a potential solution.

However, it is a tricky way and I'm not yet sure if we are supposed to do this

Isn't that just programming? Am I doing it wrong? 😅

@Nantris
Copy link
Author

Nantris commented Nov 25, 2024

Thanks again for looking into this @m-bert!

I wonder if any conclusions were reached and about any possible progress on this front. I'm hoping hard that this can be resolved, even if maybe not immediately.

Thank you again!

@m-bert
Copy link
Contributor

m-bert commented Nov 27, 2024

Hi @Nantris! We're working on it 😅

We will talk about it on Monday meeting

Unfortunately we decided that this initial approach is not something we're looking for (it could mess up things when WebView is involved, even if it wouldn't be clicked directly) 😞

However, I've found something else and maybe this will help fix interactions with WebView. I'll keep you updated!

@Nantris
Copy link
Author

Nantris commented Nov 27, 2024

Thank you so much for the update @m-bert! It means a lot to me that you're looking into this issue!

@m-bert
Copy link
Contributor

m-bert commented Nov 29, 2024

Hi again @Nantris! I have a question, could you take a look at this PR? I think it can resolve this issue with a small modification of your code, i.e.:

function WebViewScreen() {
  const native = Gesture.Native();

  return (
    <View style={styles.webViewContainer}>
      <GestureDetector gesture={native}>
        <WebView
          source={{ uri: 'https://templates.tiptap.dev/nw6Cmz6HfD' }}
          style={styles.webView}
          javaScriptEnabled={true}
          domStorageEnabled={true}
          startInLoadingState={true}
          onError={(syntheticEvent) => {
            const { nativeEvent } = syntheticEvent;
            console.warn('WebView error: ', nativeEvent);
          }}
        />
      </GestureDetector>
    </View>
  );
}

Please let me know if it works!

@Nantris
Copy link
Author

Nantris commented Nov 29, 2024

Thanks so much @m-bert! This looks really promising!

I built our app using those changes and it does indeed seem to resolve the issue!

I did notice one new issue it may be causing, but I need to dig further to ensure that's not just an artifact of our implementation. I'm noticing if there's an expanded selection and I use the back gesture, it can reset the text caret inside the webview to the start of the paragraph. I suspect this is related to the workaround since the selection indicators flash off before returning. (the image shows what I'm calling "selection indicators")

image

All in all though, even if that issue is related - that PR would be a huge improvement! I hope it can be merged in the end and doesn't cause any other issues 🤞

Thank you so much for your work on this @m-bert!

@m-bert
Copy link
Contributor

m-bert commented Dec 2, 2024

I did notice one new issue it may be causing, but I need to dig further to ensure that's not just an artifact of our implementation.

Let me know if you find something!

@m-bert m-bert self-assigned this Dec 2, 2024
@m-bert m-bert closed this as completed in f0604b0 Dec 4, 2024
@Nantris
Copy link
Author

Nantris commented Dec 4, 2024

Sorry for the lack of communication - rough week! I found no further issues and was not able to reproduce the potential issue noted above. Thank you so much for your work on this @m-bert!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: Android This issue is specific to Android Repro provided A reproduction with a snack or repo is provided
Projects
None yet
2 participants