Remove menu from the SendFeedback screenshot.
SendFeedback from the back of card menu was getting the menu stuck
in the screenshot. We need to post a task to wait for the menu to
finish animating down before taking the screenshot.
This will mean that we can no longer use the menu image to infer
if the user is signed in with WAA on. I added b:235240274 to
see if we can add that information to the PSD for the feedback.
Bug: 1225749
Change-Id: I512ca1a4f7ebcc63f12b4307d57f26e33d7af7a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3698737
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1015003}
diff --git a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedStream.java b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedStream.java
index 3026d9a..7d3321e 100644
--- a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedStream.java
+++ b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedStream.java
@@ -282,6 +282,9 @@
class FeedActionsHandlerImpl implements FeedActionsHandler {
private static final int SNACKBAR_DURATION_MS_SHORT = 4000;
private static final int SNACKBAR_DURATION_MS_LONG = 10000;
+ // This is based on the menu animation time (218ms) from BottomSheet.java.
+ // It is private to an internal target, so we can't link, to it here.
+ private static final int MENU_DISMISS_TASK_DELAY = 318;
@VisibleForTesting
static final String FEEDBACK_REPORT_TYPE =
@@ -302,9 +305,6 @@
FeedStreamJni.get().reportOtherUserAction(
mNativeFeedStream, FeedStream.this, FeedUserActionType.TAPPED_SEND_FEEDBACK);
- // Make sure the bottom sheet is dismissed before we take a snapshot.
- dismissBottomSheet();
-
Profile profile = Profile.getLastUsedRegularProfile();
if (profile == null) {
return;
@@ -314,11 +314,19 @@
Map<String, String> feedContext = convertNameFormat(productSpecificDataMap);
+ // We want to hide the bottom sheet before sending feedback so the snapshot doesn't show
+ // the menu covering the article. However the menu is animating down, we need to wait
+ // for the animation to finish. We post a task to wait for the duration of the
+ // animation, then call send feedback.
+
// FEEDBACK_REPORT_TYPE: Reports for Chrome mobile must have a contextTag of the form
// com.chrome.feed.USER_INITIATED_FEEDBACK_REPORT, or they will be discarded for not
// matching an allow list rule.
- mHelpAndFeedbackLauncher.showFeedback(
- mActivity, profile, url, FEEDBACK_REPORT_TYPE, feedContext);
+ PostTask.postDelayedTask(UiThreadTaskTraits.DEFAULT,
+ ()
+ -> mHelpAndFeedbackLauncher.showFeedback(
+ mActivity, profile, url, FEEDBACK_REPORT_TYPE, feedContext),
+ MENU_DISMISS_TASK_DELAY);
}
@Override
diff --git a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedStreamTest.java b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedStreamTest.java
index 06f975d6..4bd58be 100644
--- a/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedStreamTest.java
+++ b/chrome/browser/feed/android/java/src/org/chromium/chrome/browser/feed/FeedStreamTest.java
@@ -7,7 +7,6 @@
import static androidx.test.espresso.matcher.ViewMatchers.hasDescendant;
import static org.hamcrest.CoreMatchers.not;
-import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -776,39 +775,6 @@
@Test
@SmallTest
- public void testSendFeedback() {
- final String testUrl = TEST_URL;
- final String testTitle = "Chromium based browsers for the win!";
- final String xSurfaceCardTitle = "Card Title";
- final String cardTitle = "CardTitle";
- final String cardUrl = "CardUrl";
- // Arrange.
- Map<String, String> productSpecificDataMap = new HashMap<>();
- productSpecificDataMap.put(FeedStream.FeedActionsHandlerImpl.XSURFACE_CARD_URL, testUrl);
- productSpecificDataMap.put(xSurfaceCardTitle, testTitle);
-
- mFeedStream.setHelpAndFeedbackLauncherForTest(mHelpAndFeedbackLauncher);
- bindToView();
- FeedStream.FeedActionsHandlerImpl handler =
- (FeedStream.FeedActionsHandlerImpl) mContentManager.getContextValues(0).get(
- FeedActionsHandler.KEY);
-
- // Act.
- handler.sendFeedback(productSpecificDataMap);
-
- // Assert.
- verify(mHelpAndFeedbackLauncher)
- .showFeedback(any(), any(), eq(testUrl),
- eq(FeedStream.FeedActionsHandlerImpl.FEEDBACK_REPORT_TYPE),
- mMapCaptor.capture());
-
- // Check that the map contents are as expected.
- assertThat(mMapCaptor.getValue(), hasEntry(cardUrl, testUrl));
- assertThat(mMapCaptor.getValue(), hasEntry(cardTitle, testTitle));
- }
-
- @Test
- @SmallTest
public void testShowSnackbar() {
bindToView();
FeedStream.FeedActionsHandlerImpl handler =