New issue
Advanced search Search tips

Issue 848434 link

Starred by 2 users

Issue metadata

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

Blocking:
issue 737777



Sign in to add a comment

zoom into the focused editable doesn't work when use-zoom-for-dsf enabled

Project Member Reported by eirage@chromium.org, May 31 2018

Issue description

OS: Android

What steps will reproduce the problem?
(1) enable use-zoom-for dsf
(2) On a page with input box without viewport meta (http://output.jsbin.com/ciqucom/11/quiet)
(3) tap on the input box

What is the expected result?
page should zoom to legible scale

What happens instead?
no zoom

 

Comment 1 by eirage@chromium.org, May 31 2018

Blocking: 737777
Cc: bokan@chromium.org
SitePerProcessProgrammaticScrollTest.ScrollClippedFocusedEditableElementIntoView failed with the flag enabled

Comment 2 by bokan@chromium.org, Jun 1 2018

I came across what I think may cause this bug in an unrelated change. Hold off on this one until I land my patch and we'll see if that fixes it.

Comment 3 by bokan@chromium.org, Jun 1 2018

Cc: -bokan@chromium.org eirage@chromium.org
Owner: bokan@chromium.org

Comment 4 by bokan@chromium.org, Jun 1 2018

Cc: -eirage@chromium.org bokan@chromium.org
Owner: eirage@chromium.org
I've reduced the scope of my change so it wont affect this issue. My unconfirmed suspicion was that the Element bounds in WebViewImpl::ComputeScaleAndScrollForEditableElementRects are in CSS pixels but visual_viewport.VisibleRect() is in |physical pixels / page scale| (the latter I know for sure). 
I think Element bounds are in physical_pixel, but minReadableCaretHeight seems to be hard-coded css pixel value, so the calculated new scale is wrong :D
See:
https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/exported/web_view_impl.cc?gsn=GetWebView&g=0&l=205
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 19 2018

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

commit b65e16f8cdabf8ea35a06d7ddf8db1c02330a918
Author: Ella Ge <eirage@chromium.org>
Date: Tue Jun 19 17:30:46 2018

Fix zoom into focused editable when enable zoom-for-dsf

When zoom-for-dsf enabled, element_bound_in_document is in physical pixel;
however, minReadableCaretHeight is hard-coded css pixel value. So we need
to applied page zoom factor to the min caret height before calculate the
new page scale.

Bug:  848434 
Change-Id: I3fe529032fbc380f4dc90a42a0fef532b79ba38b
Reviewed-on: https://chromium-review.googlesource.com/1085559
Commit-Queue: Ella Ge <eirage@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568510}
[modify] https://crrev.com/b65e16f8cdabf8ea35a06d7ddf8db1c02330a918/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/b65e16f8cdabf8ea35a06d7ddf8db1c02330a918/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/b65e16f8cdabf8ea35a06d7ddf8db1c02330a918/third_party/blink/renderer/core/exported/web_view_impl.h

Comment 7 by eirage@chromium.org, Jun 21 2018

Status: Fixed (was: Assigned)

Comment 8 by ram...@chromium.org, Jun 28 2018

Bug is fixed on Chrome Dev 69.0.3475.0

Sign in to add a comment