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 pinch crash in touch-responsive views. #17167

Closed
wants to merge 1 commit into from

Conversation

tobycox
Copy link
Contributor

@tobycox tobycox commented Dec 13, 2017

Fork and rebase of @gillessed's PR #13166 which has gotten stale.

From original PR:

Motivation (required)

Multiple react native developer (including myself) have run into a crash with the react-native-photo-view library (and possibly others). The common solution to this problem lies in the underlying java code, and thus requires a change in the react native source.

The stack trace I am getting is the same as listed here alwx/react-native-photo-view#15.

There was a PR to fix this (#12085) but it was closed. In response to the comments there, in my PR, I do log the exceptions. I don't think we can get any closer to the exception because in the next level of the stack trace, we are in the android sdk code.

Looking at some stack overflow pages and the android bug tracker, it seems that this is the common solution to this bug, and does not cause any impact any functionality.

https://code.google.com/p/android/issues/list?can=1&q=pointerindex+out+of+range&colspec=ID+Status+Priority+Owner+Summary+Stars+Reporter+Opened&cells=tiles

Test Plan (required)

I have manually tested this by compiling react native android from source and have confirmed the exception still gets hit and logged, but does not cause the app to terminate.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 13, 2017
@tobycox tobycox force-pushed the bugfix/pinch-crash branch 2 times, most recently from 7fcf5d3 to 67a6abe Compare January 12, 2018 01:50
@pull-bot
Copy link

pull-bot commented Jan 12, 2018

Warnings
⚠️

📋 Release Notes - This PR appears to be missing Release Notes.

Generated by 🚫 dangerJS

@facebook-github-bot
Copy link
Contributor

@tobycox I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

@tlenclos
Copy link

Hi there,
could someone restart the circle CI job on timeout and review this ?

This issue is encountered on many android apps with pinch functionnalities and right now the only work around is to maintain a fork of React native to avoid crashing the app. I don't mind helping or fix tests if that is an issue, just tell me.

@ide ide requested a review from hramos February 15, 2018 21:15
@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 16, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Plo4ox pushed a commit to Plo4ox/react-native that referenced this pull request Feb 17, 2018
Summary:
Fork and rebase of gillessed's PR facebook#13166 which has gotten stale.

From original PR:

Motivation (required)

Multiple react native developer (including myself) have run into a crash with the react-native-photo-view library (and possibly others). The common solution to this problem lies in the underlying java code, and thus requires a change in the react native source.

The stack trace I am getting is the same as listed here alwx/react-native-photo-view#15.

There was a PR to fix this (facebook#12085) but it was closed. In response to the comments there, in my PR, I do log the exceptions. I don't think we can get any closer to the exception because in the next level of the stack trace, we are in the android sdk code.

Looking at some stack overflow pages and the android bug tracker, it seems that this is the common solution to this bug, and does not cause any impact any functionality.

https://code.google.com/p/android/issues/list?can=1&q=pointerindex+out+of+range&colspec=ID+Status+Priority+Owner+Summary+Stars+Reporter+Opened&cells=tiles

Test Plan (required)

I have manually tested this by compiling react native android from source and have confirmed the exception still gets hit and logged, but does not cause the app to terminate.
Closes facebook#17167

Differential Revision: D7014296

Pulled By: hramos

fbshipit-source-id: 06b4a31062a591b726d2021e877d16f49881dcfd
ferrannp pushed a commit to ferrannp/react-native-viewpager that referenced this pull request Feb 8, 2019
Summary:
Fork and rebase of gillessed's PR facebook/react-native#13166 which has gotten stale.

From original PR:

Motivation (required)

Multiple react native developer (including myself) have run into a crash with the react-native-photo-view library (and possibly others). The common solution to this problem lies in the underlying java code, and thus requires a change in the react native source.

The stack trace I am getting is the same as listed here alwx/react-native-photo-view#15.

There was a PR to fix this (#12085) but it was closed. In response to the comments there, in my PR, I do log the exceptions. I don't think we can get any closer to the exception because in the next level of the stack trace, we are in the android sdk code.

Looking at some stack overflow pages and the android bug tracker, it seems that this is the common solution to this bug, and does not cause any impact any functionality.

https://code.google.com/p/android/issues/list?can=1&q=pointerindex+out+of+range&colspec=ID+Status+Priority+Owner+Summary+Stars+Reporter+Opened&cells=tiles

Test Plan (required)

I have manually tested this by compiling react native android from source and have confirmed the exception still gets hit and logged, but does not cause the app to terminate.
Closes facebook/react-native#17167

Differential Revision: D7014296

Pulled By: hramos

fbshipit-source-id: 06b4a31062a591b726d2021e877d16f49881dcfd
james-watkin added a commit to james-watkin/react-native-pager-view that referenced this pull request Dec 28, 2021
Summary:
Fork and rebase of gillessed's PR facebook/react-native#13166 which has gotten stale.

From original PR:

Motivation (required)

Multiple react native developer (including myself) have run into a crash with the react-native-photo-view library (and possibly others). The common solution to this problem lies in the underlying java code, and thus requires a change in the react native source.

The stack trace I am getting is the same as listed here alwx/react-native-photo-view#15.

There was a PR to fix this (#12085) but it was closed. In response to the comments there, in my PR, I do log the exceptions. I don't think we can get any closer to the exception because in the next level of the stack trace, we are in the android sdk code.

Looking at some stack overflow pages and the android bug tracker, it seems that this is the common solution to this bug, and does not cause any impact any functionality.

https://code.google.com/p/android/issues/list?can=1&q=pointerindex+out+of+range&colspec=ID+Status+Priority+Owner+Summary+Stars+Reporter+Opened&cells=tiles

Test Plan (required)

I have manually tested this by compiling react native android from source and have confirmed the exception still gets hit and logged, but does not cause the app to terminate.
Closes facebook/react-native#17167

Differential Revision: D7014296

Pulled By: hramos

fbshipit-source-id: 06b4a31062a591b726d2021e877d16f49881dcfd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants