New issue
Advanced search Search tips

Issue 740136 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 729185



Sign in to add a comment

adjust initial overlay position

Project Member Reported by liber...@chromium.org, Jul 7 2017

Issue description

DialogOverlayImpl doesn't adjust the initial position for the android status bar etc.  it should do the same thing that it does for scheduleLayout().
 
This might also fix: 729185
Blocking: 729185
i think that 740136 must be fixed before we can fix  crbug.com/729185  .  740136 causes DialogOverlayImpl to do the wrong thing with whatever initial position we send, while 729185 causes us to send the wrong position.

i've got a fix locally for this bug.  i'm also finishing up pixel tests that will check both the initial position and scheduleLayout.  they're not great, but we kinda need 'em.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 11 2017

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

commit 251fe521d9d694d0fdcc31da527e2bb497bdaadf
Author: liberato@chromium.org <liberato@chromium.org>
Date: Tue Jul 11 00:54:06 2017

Fix AndroidOverlay position, add pixel tests.

The initial position for an overlay was not being adjusted for the
height of the status bar.  This CL adds the adjustment.

It also adds DialogOverlayImplPixelTest.java to verify that both
the initial position and updates via scheduleLayout() work as
intended.  These rely on UiAutomation, so that it can take a
screenshot that contains overlays.

Bug:  740136 
Change-Id: Ib3194869972041a39a06a2d025e5f549d1319cc4
Reviewed-on: https://chromium-review.googlesource.com/563484
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485470}
[modify] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/browser/android/dialog_overlay_impl.cc
[modify] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/browser/android/dialog_overlay_impl.h
[modify] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/public/android/BUILD.gn
[modify] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/public/android/java/src/org/chromium/content/browser/androidoverlay/AndroidOverlayProviderImpl.java
[modify] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/public/android/java/src/org/chromium/content/browser/androidoverlay/DialogOverlayCore.java
[modify] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/public/android/java/src/org/chromium/content/browser/androidoverlay/DialogOverlayImpl.java
[add] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/public/android/javatests/src/org/chromium/content/browser/androidoverlay/DialogOverlayImplPixelTest.java
[modify] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/public/android/javatests/src/org/chromium/content/browser/androidoverlay/DialogOverlayImplTest.java
[add] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/public/android/javatests/src/org/chromium/content/browser/androidoverlay/DialogOverlayImplTestBase.java
[modify] https://crrev.com/251fe521d9d694d0fdcc31da527e2bb497bdaadf/content/public/android/junit/src/org/chromium/content/browser/androidoverlay/DialogOverlayCoreTest.java

Status: Fixed (was: Started)

Sign in to add a comment