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

Fix for crash on Samsung Clipboard ListView on Android 8.X #795

Merged
merged 3 commits into from
Mar 21, 2019

Conversation

daniloercoli
Copy link
Contributor

@daniloercoli daniloercoli commented Mar 19, 2019

This PR fixes a crash that happens when pasting text on Samsung Devices running Android 8.
The issue only happens if you use the custom Samsung Clipboard feature -
Ref: wordpress-mobile/WordPress-Android#8827

Steps to reproduce:

Make sure you have Samsung device with Android 8.x

  • Start the demo app
  • Hold down finger over the screen until a menu with: PASTE, CLIPBOARD doesn't show up.
  • Click on CLIPBOARD
  • See the Toast message and that the custom list view doesn't show

The solution implemented in this PR is the less invasive, and easy to remove later, other viable fixes are listed here wordpress-mobile/WordPress-Android#8827 (comment)

Note: we will update the Aztec ref in the Android app soon, with the next GB-mobile release, so no need to update it now on both projects.

cc @marecar3 since you've already investigate the problem.

Note 2: If we find a nice way to disable HW acceleration, on selected devices only, we may need to remove this fix, and try if it's crashing again when HW acceleration is OFF. Since this could be similar to #8828.

@malinajirka
Copy link
Contributor

Thanks for the fix! Unfortunately, I have access just to a Samsung with Android 4.4. @marecar3 Could you please review this PR? Thanks!

@marecar3
Copy link
Contributor

Hey @malinajirka, yup I can do that, will respond with a proper comment.

@marecar3
Copy link
Contributor

marecar3 commented Mar 21, 2019

Hey @malinajirka @daniloercoli , On my device Samsung SM-T585 Android 8.1 this solution isn't working as it has different value for the Clipboard 16908872.

Generally not sure if it's good to put the hardcoded value from the Android platform, as it can be different on various devices (like in this case) and also if it's maybe working now, it's not guaranteed that Samsung won't change it in the future.

else -> return super.onTextContextMenuItem(id)
// Fix for crash when pasting text on Samsung Devices running Android 8.
// Ref: https://github.com/wordpress-mobile/WordPress-Android/issues/8827
16908904,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value on my device : Samsung SM-T585 Android 8.1 is different -> 16908872.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting.... 16908874 --> was from Android 8, and 16908904 from Android 9. I just added it since don't know if 8.X will get the latest version of their clipboard.

I guess i can add your other value to the list, but it's like going per-case. Anyway don't have better idea, since the src code of their clipboard isn't available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daniloercoli let me try something, and if doesn't work I will give the Approve so that we can move on.

@marecar3
Copy link
Contributor

Hey @daniloercoli if you don't mind I have changed initial solution and put some dynamic solution now, it's working on my device, please can you try on your device?

@daniloercoli
Copy link
Contributor Author

Nice improvement @marecar3! Worked as expected!

Copy link
Contributor

@marecar3 marecar3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! You can merge it @daniloercoli if you want.

@daniloercoli daniloercoli merged commit 7435c6c into develop Mar 21, 2019
@daniloercoli daniloercoli deleted the issue/fix-for-samsung-custom-cliboard-android-8 branch March 21, 2019 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants