New issue
Advanced search Search tips

Issue 776607 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 417782



Sign in to add a comment

[root layer scrolls] elementsFromPoint layout tests are failing

Project Member Reported by pdr@chromium.org, Oct 20 2017

Issue description

The following tests are failing with root layer scrolling:
fast/dom/elementsFromPoint/elementsFromPoint-iframes.html
fast/dom/elementsFromPoint/elementsFromPoint-invalid-cases.html
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 23 2017

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

commit 9960cbaff4e136069ce2c3026d747b7679c8ef29
Author: Philip Rogers <pdr@chromium.org>
Date: Mon Oct 23 23:45:42 2017

Ensure scrollbars are laid out before hit testing

This patch fixes a bug in hit testing where the frame was not
laid out before hit testing scrollbars. Layout is required to
ensure scrollbars are created/destroyed to account for content.

Bug:  776607 
Change-Id: I4523d731d707de4ec3ffcf8721a86a32e68c464f
Reviewed-on: https://chromium-review.googlesource.com/733620
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510961}
[modify] https://crrev.com/9960cbaff4e136069ce2c3026d747b7679c8ef29/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/9960cbaff4e136069ce2c3026d747b7679c8ef29/third_party/WebKit/Source/core/dom/TreeScope.cpp
[modify] https://crrev.com/9960cbaff4e136069ce2c3026d747b7679c8ef29/third_party/WebKit/Source/core/dom/TreeScope.h

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 24 2017

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

commit f8350aeae266267a394fd462fe83131ec2818b7b
Author: Yuta Kitamura <yutak@chromium.org>
Date: Tue Oct 24 05:22:13 2017

Revert "Ensure scrollbars are laid out before hit testing"

This reverts commit 9960cbaff4e136069ce2c3026d747b7679c8ef29.

Reason for revert: Caused test failures in webkit_unit_test

DocumentTest.ElementFromPointOnScrollbar
UseCounterTest.RecordingExtensions (?)
UseCounterTest.SVGImageContextFeatures (?)
(The last two may be irrelevant to your change.)

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Android%20(Nexus4)

I  162.788s run_tests_on_device(0273818f0c5cabb6)  [ RUN      ] DocumentTest.ElementFromPointOnScrollbar
I  162.788s run_tests_on_device(0273818f0c5cabb6)  ../../third_party/WebKit/Source/core/dom/DocumentTest.cpp:964: Failure
I  162.788s run_tests_on_device(0273818f0c5cabb6)        Expected: GetDocument().ElementFromPoint(1, 590)
I  162.788s run_tests_on_device(0273818f0c5cabb6)        Which is: BODY
I  162.788s run_tests_on_device(0273818f0c5cabb6)  To be equal to: nullptr
I  162.788s run_tests_on_device(0273818f0c5cabb6)        Which is: 4-byte object <00-00 00-00>
I  162.788s run_tests_on_device(0273818f0c5cabb6)  [  FAILED  ] DocumentTest.ElementFromPointOnScrollbar (9 ms)

Original change's description:
> Ensure scrollbars are laid out before hit testing
> 
> This patch fixes a bug in hit testing where the frame was not
> laid out before hit testing scrollbars. Layout is required to
> ensure scrollbars are created/destroyed to account for content.
> 
> Bug:  776607 
> Change-Id: I4523d731d707de4ec3ffcf8721a86a32e68c464f
> Reviewed-on: https://chromium-review.googlesource.com/733620
> Commit-Queue: Philip Rogers <pdr@chromium.org>
> Reviewed-by: Steve Kobes <skobes@chromium.org>
> Reviewed-by: Stefan Zager <szager@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510961}

TBR=szager@chromium.org,pdr@chromium.org,skobes@chromium.org

Change-Id: I893f56046e621c01ac9fa1e4bd1a4eaba03b90db
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  776607 
Reviewed-on: https://chromium-review.googlesource.com/735239
Reviewed-by: Yuta Kitamura <yutak@chromium.org>
Commit-Queue: Yuta Kitamura <yutak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511043}
[modify] https://crrev.com/f8350aeae266267a394fd462fe83131ec2818b7b/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/f8350aeae266267a394fd462fe83131ec2818b7b/third_party/WebKit/Source/core/dom/TreeScope.cpp
[modify] https://crrev.com/f8350aeae266267a394fd462fe83131ec2818b7b/third_party/WebKit/Source/core/dom/TreeScope.h

Comment 3 by aluo@chromium.org, Oct 24 2017

Labels: chromium-waterfall
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 24 2017

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

commit 4fc60db274fcd51069d5830b112c3bc76e4234e4
Author: Philip Rogers <pdr@chromium.org>
Date: Tue Oct 24 22:17:26 2017

[reland] Ensure scrollbars are laid out before hit testing

This patch fixes a bug in hit testing where the frame was not
laid out before hit testing scrollbars. Layout is required to
ensure scrollbars are created/destroyed to account for content.

This is a reland of [1] which was rolled out due to failing
on Android. This is because Android does not support top-level
non-overlay scrollbars.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/733620

Bug:  776607 
Change-Id: I7b43b5aa516930ad88fd94594978c10e606a1666
Reviewed-on: https://chromium-review.googlesource.com/735513
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511279}
[modify] https://crrev.com/4fc60db274fcd51069d5830b112c3bc76e4234e4/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/4fc60db274fcd51069d5830b112c3bc76e4234e4/third_party/WebKit/Source/core/dom/TreeScope.cpp
[modify] https://crrev.com/4fc60db274fcd51069d5830b112c3bc76e4234e4/third_party/WebKit/Source/core/dom/TreeScope.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 26 2017

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

commit fbc3c374d9528c25541d912dbe0e17206c56615e
Author: Philip Rogers <pdr@chromium.org>
Date: Wed Oct 25 23:59:04 2017

[root layer scrolls] Make TreeScope hit testing RLS-aware

This patch updates TreeScope's PointWithScrollAndZoomIfPossible
to work with root layer scrolling (RLS) where the layout
viewport scrollable area should be used to access scroll
offset. This fixes 2 layout tests with RLS.

Bug:  776607 
Change-Id: Ib4a96ead3565b2975ba3c1f21703021a15229abe
Reviewed-on: https://chromium-review.googlesource.com/729750
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511651}
[modify] https://crrev.com/fbc3c374d9528c25541d912dbe0e17206c56615e/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/fbc3c374d9528c25541d912dbe0e17206c56615e/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/fbc3c374d9528c25541d912dbe0e17206c56615e/third_party/WebKit/Source/core/dom/TreeScope.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 26 2017

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

commit 4903d93d2f7f315019868cdc863391a3dea834fc
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Thu Oct 26 08:25:21 2017

Revert "[root layer scrolls] Make TreeScope hit testing RLS-aware"

This reverts commit fbc3c374d9528c25541d912dbe0e17206c56615e.

Reason for revert: The test All/ParameterizedDocumentTest.DISABLE_ON_ANDROID(ElementFromPointOnScrollbar) surprisingly fails on Android :)

Original change's description:
> [root layer scrolls] Make TreeScope hit testing RLS-aware
> 
> This patch updates TreeScope's PointWithScrollAndZoomIfPossible
> to work with root layer scrolling (RLS) where the layout
> viewport scrollable area should be used to access scroll
> offset. This fixes 2 layout tests with RLS.
> 
> Bug:  776607 
> Change-Id: Ib4a96ead3565b2975ba3c1f21703021a15229abe
> Reviewed-on: https://chromium-review.googlesource.com/729750
> Commit-Queue: Philip Rogers <pdr@chromium.org>
> Reviewed-by: Steve Kobes <skobes@chromium.org>
> Reviewed-by: Stefan Zager <szager@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#511651}

TBR=szager@chromium.org,pdr@chromium.org,skobes@chromium.org

Change-Id: I0d196407e7c05835ea1241fb5eb99e016d224316
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  776607 
Reviewed-on: https://chromium-review.googlesource.com/738038
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511765}
[modify] https://crrev.com/4903d93d2f7f315019868cdc863391a3dea834fc/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/4903d93d2f7f315019868cdc863391a3dea834fc/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/4903d93d2f7f315019868cdc863391a3dea834fc/third_party/WebKit/Source/core/dom/TreeScope.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 26 2017

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

commit 2627ad21a36df310d95fe595b1cc6acc018aba91
Author: Philip Rogers <pdr@chromium.org>
Date: Thu Oct 26 18:10:08 2017

[root layer scrolls] Make TreeScope hit testing RLS-aware

This patch updates TreeScope's PointWithScrollAndZoomIfPossible
to work with root layer scrolling (RLS) where the layout
viewport scrollable area should be used to access scroll
offset. This fixes 2 layout tests with RLS.

This is a reland of [1] which caused ElementFromPointOnScrollbar
to fail on Android because the nested macro expansion did not
work. This patch replaces the macro with a simple ifdef for
Android.

[1] https://chromium.googlesource.com/chromium/src/+/fbc3c374d9528c25541d912dbe0e17206c56615e

Bug:  776607 
Change-Id: I6f66b0c1d6f3ab046e8ef6681744dca55bc82fc0
Reviewed-on: https://chromium-review.googlesource.com/739757
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511879}
[modify] https://crrev.com/2627ad21a36df310d95fe595b1cc6acc018aba91/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/2627ad21a36df310d95fe595b1cc6acc018aba91/third_party/WebKit/Source/core/dom/DocumentTest.cpp
[modify] https://crrev.com/2627ad21a36df310d95fe595b1cc6acc018aba91/third_party/WebKit/Source/core/dom/TreeScope.cpp

Comment 8 by pdr@chromium.org, Oct 26 2017

Status: Fixed (was: Assigned)

Sign in to add a comment