New issue
Advanced search Search tips

Issue 628436 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[TTS] A Context menu should hide the Contextual Search UX

Project Member Reported by donnd@chromium.org, Jul 15 2016

Issue description

When the CS UX is showing as a peeking bar, the user can still interact with the page and cause a context menu to appear (by longpressing on a link or image).  Currently CS gets various selection events that confuse the selection controller so it converts a tap-select into a longpress select.  Instead it should just hide the UX.

This sequence is implicated in a crash -- see issue 562353.  Fixing this may not fix the crash because it could be due to a broader issue.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b09e3678e18b7d6447e1c758263d290b20b18a7f

commit b09e3678e18b7d6447e1c758263d290b20b18a7f
Author: donnd <donnd@chromium.org>
Date: Fri Jul 15 23:42:10 2016

[TTS] Handle showing of a Context Menu more gracefully.

When the TTS UX is showing and the user activates a context menu,
the selection controller gets an event sequence that's confusing,
causing a tap-selection to show the selection handles.

This change hides the Contextual Search UX, and adds a state where
the selection controller knows to ignore the next event that would
show the selection handles, when a context menu has just been shown.

This change cleans up a sequence implicated in a crash, so that
won't be reproducible through triggering the Context Menu anymore.
(Issue 562353)

BUG= 628436 ,562353

Review-Url: https://codereview.chromium.org/2150213002
Cr-Commit-Position: refs/heads/master@{#405901}

[modify] https://crrev.com/b09e3678e18b7d6447e1c758263d290b20b18a7f/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/b09e3678e18b7d6447e1c758263d290b20b18a7f/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/b09e3678e18b7d6447e1c758263d290b20b18a7f/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTabHelper.java

Comment 2 by donnd@chromium.org, Jul 16 2016

Status: Fixed (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 16 2016

Labels: merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8523f7f3ca27cb7465acc94e4ab2fde5c35b8f87

commit 8523f7f3ca27cb7465acc94e4ab2fde5c35b8f87
Author: Donn Denman <donnd@google.com>
Date: Sat Jul 16 06:09:32 2016

[TTS] Handle showing of a Context Menu more gracefully.

When the TTS UX is showing and the user activates a context menu,
the selection controller gets an event sequence that's confusing,
causing a tap-selection to show the selection handles.

This change hides the Contextual Search UX, and adds a state where
the selection controller knows to ignore the next event that would
show the selection handles, when a context menu has just been shown.

This change cleans up a sequence implicated in a crash, so that
won't be reproducible through triggering the Context Menu anymore.
(Issue 562353)

BUG= 628436 ,562353

Review-Url: https://codereview.chromium.org/2150213002
Cr-Commit-Position: refs/heads/master@{#405901}
(cherry picked from commit b09e3678e18b7d6447e1c758263d290b20b18a7f)

Review URL: https://codereview.chromium.org/2153353002 .

Cr-Commit-Position: refs/branch-heads/2743@{#655}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/8523f7f3ca27cb7465acc94e4ab2fde5c35b8f87/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/8523f7f3ca27cb7465acc94e4ab2fde5c35b8f87/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/8523f7f3ca27cb7465acc94e4ab2fde5c35b8f87/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTabHelper.java

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18 2016

Labels: merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc73bd54293f704974c46c7c5043d50b98315eee

commit fc73bd54293f704974c46c7c5043d50b98315eee
Author: Alex Mineer <amineer@chromium.org>
Date: Mon Jul 18 18:30:04 2016

[TTS] Handle showing of a Context Menu more gracefully.

When the TTS UX is showing and the user activates a context menu,
the selection controller gets an event sequence that's confusing,
causing a tap-selection to show the selection handles.

This change hides the Contextual Search UX, and adds a state where
the selection controller knows to ignore the next event that would
show the selection handles, when a context menu has just been shown.

This change cleans up a sequence implicated in a crash, so that
won't be reproducible through triggering the Context Menu anymore.
(Issue 562353)

BUG= 628436 ,562353

Review-Url: https://codereview.chromium.org/2150213002
Cr-Commit-Position: refs/heads/master@{#405901}
(cherry picked from commit b09e3678e18b7d6447e1c758263d290b20b18a7f)

Review URL: https://codereview.chromium.org/2159923002 .

Cr-Commit-Position: refs/branch-heads/2785@{#189}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/fc73bd54293f704974c46c7c5043d50b98315eee/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManager.java
[modify] https://crrev.com/fc73bd54293f704974c46c7c5043d50b98315eee/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/fc73bd54293f704974c46c7c5043d50b98315eee/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchTabHelper.java

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae01e3f319b3d133c7c54c84519dc753b4922cf2

commit ae01e3f319b3d133c7c54c84519dc753b4922cf2
Author: donnd <donnd@chromium.org>
Date: Wed Jun 28 18:24:05 2017

[TTS] Fix assert after TTS and Context Menu.

ContextualSearchSelectionController was checking whether the context menu
had been shown to sort out confusing event sequences.  This is
causing the CSM to assert because the selection type is not set up
when a long-press follows Context Menu dismissal.

BUG= 737285 , 628436 

Review-Url: https://codereview.chromium.org/2957213002
Cr-Commit-Position: refs/heads/master@{#483068}

[modify] https://crrev.com/ae01e3f319b3d133c7c54c84519dc753b4922cf2/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java

Sign in to add a comment