New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 705841 link

Starred by 1 user
Status: Fixed
Owner:
█ mgiuca@chromium.org
Long OOO (go/where-is-mgiuca)
Closed: Mar 2017
Cc:
tedc...@chromium.org
mdjones@chromium.org
khushals...@chromium.org
dtrainor@chromium.org
pkasting@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Android: Tab strip isn't working / interactive

Project Member Reported by mgiuca@chromium.org, Mar 28 2017 
Chrome Version: 59 (r459989) developer build from ToT
OS: Android

What steps will reproduce the problem?
(1) Touch an inactive tab in the tab strip.
(2) Drag tabs in the tab strip.
(3) Press the X in the tab strip.
(4) Press the New Tab button.

What is the expected result?
They work.

What happens instead?
None of the above things have any effect.

Confirmed that this was working in the latest canary (r459685), and a developer build of the same version is also working (so this is not a configuration problem).

Now bisecting (only 304 revisions so should not be too bad).

Comment 1 by mgiuca@chromium.org, Mar 28 2017

Cc: mdjones@chromium.org dtrainor@chromium.org khushals...@chromium.org
Found culprit from bisecting: r459980 / https://codereview.chromium.org/2774443002
(This CL alters the tab strip event logic in Android so makes sense.)

Reverting.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 28 2017 

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

commit 6cbf12a1814e63a869a21890199f3a7babf51867
Author: mgiuca <mgiuca@chromium.org>
Date: Tue Mar 28 05:54:52 2017

Revert of chrome/android: Push EventFilters into the Layout implementations. (patchset #5 id:80001 of https://codereview.chromium.org/2774443002/ )

Reason for revert:
Made Android tabstrip completely unresponsive to touch.

BUG= 705841 

Original issue's description:
> chrome/android: Push EventFilters into the Layout implementations.
>
> EventFilters are currently built by the LayoutManager and all gestures
> are routed through it to the active layout. This indirection is
> unnecessary since only the StackLayout requires gesture handling. This
> also ends up exposing public APIs on the Layouts making it difficult to
> follow the path for events from different sources (CompositorView vs
> the Toolbar).
>
> This change pushes EventFilters deeper into the Layout implementations,
> so the Layouts directly consume touch events and generate gestures if
> necessary. Layout still has an API for swipe gestures coming from the
> toolbar.
>
> BUG=
>
> Review-Url: https://codereview.chromium.org/2774443002
> Cr-Commit-Position: refs/heads/master@{#459980}
> Committed: https://chromium.googlesource.com/chromium/src/+/ef25f0df9d626028752de7d945d89abe15d39dfb

TBR=dtrainor@chromium.org,mdjones@chromium.org,khushalsagar@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

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

[add] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/GestureHandlerLayoutDelegate.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/Layout.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChrome.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromePhone.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromeTablet.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/StaticLayout.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/ToolbarSwipeLayout.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/SimpleAnimationLayout.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/phone/StackLayout.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelperManager.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java/src/org/chromium/chrome/browser/widget/OverviewListLayout.java
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/android/java_sources.gni
[modify] https://crrev.com/6cbf12a1814e63a869a21890199f3a7babf51867/chrome/test/android/javatests/src/org/chromium/chrome/test/util/TabStripUtils.java

Comment 3 by mgiuca@chromium.org, Mar 28 2017

Status: Fixed (was: Started)
►

Sign in to add a comment