-
-
Notifications
You must be signed in to change notification settings - Fork 475
feat(feedback): Add screenshot attachment button to user feedback widget #5828
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
base: main
Are you sure you want to change the base?
Changes from all commits
b1ceb7f
ff9326d
bcfe6d6
ca30932
f9edb84
06702a3
a4e341a
c8af66a
dabb043
c8c08c1
0a3d51d
eb9d1ed
9411cc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package io.sentry.android.core; | ||
|
|
||
| import android.app.Activity; | ||
| import android.net.Uri; | ||
| import androidx.activity.ComponentActivity; | ||
| import androidx.activity.result.ActivityResultLauncher; | ||
| import androidx.activity.result.PickVisualMediaRequest; | ||
| import androidx.activity.result.contract.ActivityResultContracts; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
||
| /** | ||
| * Launches the androidx photo picker to attach an screenshot to user feedback. All | ||
| * androidx.activity references are isolated in this class, so it must only be loaded once {@link | ||
| * SentryUserFeedbackForm#isScreenshotPickerAvailable} has returned true. That check deliberately | ||
| * lives outside of this class, so that it can run without linking any androidx.activity type. | ||
| */ | ||
| final class SentryFeedbackScreenshotPicker { | ||
|
|
||
| interface OnScreenshotPicked { | ||
| void onScreenshotPicked(@NotNull Uri uri); | ||
| } | ||
|
|
||
| private final @NotNull ActivityResultLauncher<PickVisualMediaRequest> launcher; | ||
|
|
||
| private SentryFeedbackScreenshotPicker( | ||
| final @NotNull ActivityResultLauncher<PickVisualMediaRequest> launcher) { | ||
| this.launcher = launcher; | ||
| } | ||
|
|
||
| static @Nullable SentryFeedbackScreenshotPicker register( | ||
| final @NotNull Activity activity, | ||
| final @NotNull SentryFeedbackScreenshotPicker.OnScreenshotPicked callback) { | ||
| if (!(activity instanceof ComponentActivity)) { | ||
| return null; | ||
| } | ||
| final @NotNull ActivityResultLauncher<PickVisualMediaRequest> launcher = | ||
| ((ComponentActivity) activity) | ||
| .getActivityResultRegistry() | ||
| .register( | ||
| "sentry_user_feedback_screenshot_picker", | ||
| new ActivityResultContracts.PickVisualMedia(), | ||
| (@Nullable Uri uri) -> { | ||
| if (uri != null) { | ||
| callback.onScreenshotPicked(uri); | ||
| } | ||
| }); | ||
| return new SentryFeedbackScreenshotPicker(launcher); | ||
| } | ||
|
|
||
| void launch() { | ||
| launcher.launch( | ||
| new PickVisualMediaRequest.Builder() | ||
| .setMediaType(ActivityResultContracts.PickVisualMedia.ImageOnly.INSTANCE) | ||
| .build()); | ||
| } | ||
|
|
||
| void unregister() { | ||
| launcher.unregister(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,17 +3,24 @@ | |
| import android.app.Activity; | ||
| import android.app.AlertDialog; | ||
| import android.app.Application; | ||
| import android.content.ContentResolver; | ||
| import android.content.Context; | ||
| import android.content.ContextWrapper; | ||
| import android.database.Cursor; | ||
| import android.net.Uri; | ||
| import android.os.Bundle; | ||
| import android.provider.OpenableColumns; | ||
| import android.view.View; | ||
| import android.view.Window; | ||
| import android.view.WindowManager; | ||
| import android.webkit.MimeTypeMap; | ||
| import android.widget.Button; | ||
| import android.widget.EditText; | ||
| import android.widget.ImageView; | ||
| import android.widget.TextView; | ||
| import android.widget.Toast; | ||
| import io.sentry.Attachment; | ||
| import io.sentry.Hint; | ||
| import io.sentry.IScopes; | ||
| import io.sentry.Sentry; | ||
| import io.sentry.SentryFeedbackOptions; | ||
|
|
@@ -23,6 +30,11 @@ | |
| import io.sentry.protocol.Feedback; | ||
| import io.sentry.protocol.SentryId; | ||
| import io.sentry.protocol.User; | ||
| import io.sentry.util.ExceptionUtils; | ||
| import io.sentry.util.FileUtils; | ||
| import io.sentry.util.LoadClass; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.lang.ref.WeakReference; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
|
|
@@ -39,6 +51,9 @@ public class SentryUserFeedbackForm extends AlertDialog { | |
| private @Nullable SentryShakeDetector shakeDetector; | ||
| private @Nullable Application.ActivityLifecycleCallbacks shakeLifecycleCallbacks; | ||
|
|
||
| private @Nullable SentryFeedbackScreenshotPicker screenshotPicker; | ||
| private @Nullable Uri selectedImageUri; | ||
|
|
||
| SentryUserFeedbackForm( | ||
| final @NotNull Context context, | ||
| final int themeResId, | ||
|
|
@@ -191,6 +206,31 @@ protected void onCreate(Bundle savedInstanceState) { | |
| findViewById(R.id.sentry_dialog_user_feedback_edt_description); | ||
| final @NotNull Button btnSend = findViewById(R.id.sentry_dialog_user_feedback_btn_send); | ||
| final @NotNull Button btnCancel = findViewById(R.id.sentry_dialog_user_feedback_btn_cancel); | ||
| final @NotNull Button btnAddScreenshot = | ||
| findViewById(R.id.sentry_dialog_user_feedback_btn_add_screenshot); | ||
|
|
||
| // The button is made visible in onStart, once the screenshot picker is registered successfully | ||
| btnAddScreenshot.setVisibility(View.GONE); | ||
| btnAddScreenshot.setOnClickListener( | ||
| v -> { | ||
| if (selectedImageUri == null) { | ||
| if (screenshotPicker != null) { | ||
| try { | ||
| screenshotPicker.launch(); | ||
| } catch (Throwable t) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. gonna make the same comment that I did for the other usages of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to using the newly added |
||
| ExceptionUtils.rethrowIfFatal(t); | ||
| // e.g. no photo picker on the device, or the host activity is already gone | ||
| Sentry.getCurrentScopes() | ||
| .getOptions() | ||
| .getLogger() | ||
| .log(SentryLevel.ERROR, "Failed to launch the screenshot picker.", t); | ||
| } | ||
| } | ||
| } else { | ||
| selectedImageUri = null; | ||
| btnAddScreenshot.setText(feedbackOptions.getAddScreenshotButtonLabel()); | ||
| } | ||
| }); | ||
|
|
||
| if (feedbackOptions.isShowBranding()) { | ||
| imgLogo.setVisibility(View.VISIBLE); | ||
|
|
@@ -276,7 +316,9 @@ protected void onCreate(Bundle savedInstanceState) { | |
| } | ||
|
|
||
| // Capture the feedback. If the ID is empty, it means that the feedback was not sent | ||
| final @NotNull SentryId id = Sentry.feedback().capture(feedback); | ||
| final @NotNull Hint hint = new Hint(); | ||
| maybeAddImageAttachment(hint); | ||
| final @NotNull SentryId id = Sentry.feedback().capture(feedback, hint); | ||
| if (!id.equals(SentryId.EMPTY_ID)) { | ||
| Toast.makeText( | ||
| getContext(), feedbackOptions.getSuccessMessageText(), Toast.LENGTH_SHORT) | ||
|
|
@@ -331,6 +373,7 @@ protected void onStart() { | |
| edtMessage.setError(null); | ||
|
|
||
| final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions(); | ||
| maybeRegisterScreenshotPicker(options); | ||
| final @NotNull SentryFeedbackOptions feedbackOptions = options.getFeedbackOptions(); | ||
| final @Nullable Runnable onFormOpen = feedbackOptions.getOnFormOpen(); | ||
| if (onFormOpen != null) { | ||
|
|
@@ -340,6 +383,146 @@ protected void onStart() { | |
| currentReplayId = options.getReplayController().getReplayId(); | ||
| } | ||
|
|
||
| @Override | ||
| protected void onStop() { | ||
| super.onStop(); | ||
| if (screenshotPicker != null) { | ||
| screenshotPicker.unregister(); | ||
| screenshotPicker = null; | ||
| } | ||
| } | ||
|
|
||
| private void maybeRegisterScreenshotPicker(final @NotNull SentryOptions options) { | ||
| // Clear any previously selected image so subsequent show() calls start with a fresh form | ||
| final @NotNull Button btnAddScreenshot = | ||
| findViewById(R.id.sentry_dialog_user_feedback_btn_add_screenshot); | ||
| selectedImageUri = null; | ||
| btnAddScreenshot.setText(resolvedFeedbackOptions.getAddScreenshotButtonLabel()); | ||
|
|
||
| if (!resolvedFeedbackOptions.isEnableAttachScreenshot()) { | ||
| return; | ||
| } | ||
| final @Nullable Activity activity = getActivity(getContext()); | ||
| if (activity != null && isScreenshotPickerAvailable(options)) { | ||
| screenshotPicker = | ||
| SentryFeedbackScreenshotPicker.register( | ||
| activity, uri -> onScreenshotPicked(options, btnAddScreenshot, uri)); | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| if (screenshotPicker != null) { | ||
| btnAddScreenshot.setVisibility(View.VISIBLE); | ||
| } else { | ||
| btnAddScreenshot.setVisibility(View.GONE); | ||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.WARNING, | ||
| "Feedback screenshot button won't be shown. It requires the androidx.activity " | ||
| + "dependency and the feedback form being shown from a ComponentActivity."); | ||
|
sentry[bot] marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Must be called before {@link SentryFeedbackScreenshotPicker} is touched for the first time, as | ||
| * that class links against androidx.activity types. | ||
| */ | ||
| private boolean isScreenshotPickerAvailable(final @NotNull SentryOptions options) { | ||
| final @NotNull LoadClass loadClass = resolvedFeedbackOptions.getLoadClass(); | ||
| return loadClass.isClassAvailable("androidx.activity.ComponentActivity", options) | ||
| && loadClass.isClassAvailable( | ||
| "androidx.activity.result.contract.ActivityResultContracts$PickVisualMedia", options); | ||
| } | ||
|
|
||
| private void onScreenshotPicked( | ||
| final @NotNull SentryOptions options, | ||
| final @NotNull Button btnAddScreenshot, | ||
| final @NotNull Uri uri) { | ||
| final long size = getUriSize(options, getContext().getContentResolver(), uri); | ||
| if (size > options.getMaxAttachmentSize()) { | ||
|
sentry[bot] marked this conversation as resolved.
|
||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.WARNING, | ||
| "Selected screenshot is larger than the maxAttachmentSize of %d bytes, dropping it.", | ||
| options.getMaxAttachmentSize()); | ||
| Toast.makeText( | ||
| getContext(), | ||
| resolvedFeedbackOptions.getScreenshotTooLargeMessageText(), | ||
| Toast.LENGTH_SHORT) | ||
| .show(); | ||
| return; | ||
| } | ||
| selectedImageUri = uri; | ||
| btnAddScreenshot.setText(resolvedFeedbackOptions.getRemoveScreenshotButtonLabel()); | ||
| } | ||
|
|
||
| private void maybeAddImageAttachment(final @NotNull Hint hint) { | ||
| final @Nullable Uri imageUri = selectedImageUri; | ||
| if (imageUri == null) { | ||
| return; | ||
| } | ||
| try { | ||
| final @NotNull ContentResolver resolver = getContext().getContentResolver(); | ||
| final @Nullable String resolvedMime = resolver.getType(imageUri); | ||
| final @NotNull String mime = resolvedMime != null ? resolvedMime : "image/png"; | ||
| final @Nullable String resolvedExt = | ||
| MimeTypeMap.getSingleton().getExtensionFromMimeType(mime); | ||
| final @NotNull String ext = resolvedExt != null ? resolvedExt : "png"; | ||
| hint.addAttachment( | ||
| new Attachment( | ||
| () -> | ||
| readUriBytes( | ||
| resolver, | ||
| imageUri, | ||
| Sentry.getCurrentScopes().getOptions().getMaxAttachmentSize()), | ||
| "screenshot." + ext, | ||
| mime, | ||
| "event.attachment", | ||
| false)); | ||
|
markushi marked this conversation as resolved.
|
||
| } catch (Throwable t) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided I'm always going to comment whenever we add new usages of this. Sorry to bring it up again.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to using the newly added |
||
| ExceptionUtils.rethrowIfFatal(t); | ||
| // e.g. the read permission for the picked Uri was revoked in the meantime | ||
| Sentry.getCurrentScopes() | ||
| .getOptions() | ||
| .getLogger() | ||
| .log(SentryLevel.ERROR, "Failed to attach image to feedback.", t); | ||
| } | ||
| } | ||
|
|
||
| private static long getUriSize( | ||
| final @NotNull SentryOptions options, | ||
| final @NotNull ContentResolver resolver, | ||
| final @NotNull Uri uri) { | ||
| try (final @Nullable Cursor cursor = resolver.query(uri, null, null, null, null)) { | ||
| if (cursor != null && cursor.moveToFirst()) { | ||
| final int sizeIndex = cursor.getColumnIndex(OpenableColumns.SIZE); | ||
| if (sizeIndex != -1 && !cursor.isNull(sizeIndex)) { | ||
| return cursor.getLong(sizeIndex); | ||
| } | ||
| } | ||
| } catch (Throwable t) { | ||
| ExceptionUtils.rethrowIfFatal(t); | ||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.WARNING, | ||
| "Unable to determine the size of the selected screenshot, the attachment size limit " | ||
| + "is applied when the feedback is captured.", | ||
| t); | ||
| } | ||
| return -1; | ||
| } | ||
|
|
||
| private static byte[] readUriBytes( | ||
| final @NotNull ContentResolver resolver, final @NotNull Uri uri, final long maxSize) | ||
| throws IOException { | ||
| try (final @Nullable InputStream inputStream = resolver.openInputStream(uri)) { | ||
| if (inputStream == null) { | ||
| throw new IOException("Unable to open image attachment: " + uri); | ||
| } | ||
| return FileUtils.inputStreamToByteArray(inputStream, maxSize); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void show() { | ||
| // If Sentry is disabled, don't show the dialog, but log a warning | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The
ActivityResultRegistry.register()call for the screenshot picker is made after the Activity's lifecycle isSTARTED, which will throw an unhandledIllegalStateExceptionand crash the app.Severity: CRITICAL
Suggested Fix
The registration of the
ActivityResultLaunchershould be moved to a point in the lifecycle before the host Activity is started, such as in theonCreate()method of the Activity or Fragment. If registration must happen dynamically, wrap the call in atry-catchblock to handle theIllegalStateExceptiongracefully, although this would prevent the feature from working. The recommended approach is to register early in the lifecycle.Prompt for AI Agent
Also affects:
sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java:385~385