New issue
Advanced search Search tips

Issue 810977 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 417782



Sign in to add a comment

[RLS] transforms/selection-bounds-in-transformed-view.html broken

Project Member Reported by chrishtr@chromium.org, Feb 10 2018

Issue description

Output:

PASS FAIL (scrollTop:1120)
target

Looks like a visual viewport bug together with page zoom?


 
Blocking: 417782
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 12 2018

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

commit c9a72be183bd775d07b537414648a22316eba8b3
Author: David Bokan <bokan@chromium.org>
Date: Mon Feb 12 22:30:29 2018

Fix selection-bounds test for root-layer-scrolls.

This test revealed a bug with FrameSelection::RevealSelection. This
method calls ComputeRectToScroll to determine the rect to scroll into
view. It then calls LayoutObject::ScrollRectToVisible() on this rect to
bring it into view. The bug is that ScrollRectToVisible expectes the
rect in absolute coordinates but ComputeRectToScroll returns a rect in
document coordinates.

Prior to root-layer-scrolls, absolute and document coordinates were
equivalent. After turning on root-layer-scrolls they differ, since the
LayoutView has a scrolling box. For more details, see:
https://www.chromium.org/developers/design-documents/blink-coordinate-spaces


This CL fixes the issue by making the lower level
LayoutObject::SelectionRect method return the rect in absolute
coordinates. I've also renamed this method and other methods that use it
to clarify what coordinate space returned rects are in.

I also updated the test to make it more idiomatic with modern Blink
tests as well as well as to improve it by starting both scroll into view
checks from the same, non-origin, scroll offset.

Bug:  810977 
Change-Id: I7466c5b3b664a623358e8465c44ca0e2f3a9a215
Reviewed-on: https://chromium-review.googlesource.com/912698
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536216}
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/1f494356e38b755bc14e0eb043e28e7bb1cd9d3b/third_party/WebKit/LayoutTests/transforms/selection-bounds-in-transformed-view-expected.txt
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/LayoutTests/transforms/selection-bounds-in-transformed-view.html
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/editing/FrameSelection.h
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/editing/LayoutSelection.h
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/frame/WebLocalFrameImpl.cpp
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/layout/LayoutObject.h
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/page/DragController.cpp
[modify] https://crrev.com/c9a72be183bd775d07b537414648a22316eba8b3/third_party/WebKit/Source/core/testing/Internals.cpp

Comment 3 by bokan@chromium.org, Feb 12 2018

Status: Fixed (was: Assigned)

Sign in to add a comment