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

Regression:'Live Ticker Matches'Bar scroll along with page .

Reported by shruti.j...@etouch.net, Apr 30 2018

Issue description

Chrome Version : 68.0.3415.0 (Official Build) afa8bfeb1ad8a79157f12a1add56654e93d897c5-refs/heads/master@{#554687} 32/64-bit
OS:Mac OS X(10.12.6,10.13.1,10.13.5)

Test URL:https://www.transfermarkt.com/ticker/index/live

Steps to reproduce:
1.Launch chrome and navigate to above URL.
2.Scroll down using Touch-pad and observe the 'Live ticker Matches' bar.

Actual Result : 'Live Ticker Matches'Bar scroll along with page .
Expected Result :'Live Ticker Matches'Bar should not get scroll along with page and should stay according to its position on page.

This is regression issue broken in ‘M-68’ and below is the bisect using old script:

Good Build: 68.0.3406.0(Revision:553302) 
Bad Build:  68.0.3409.0Revision:553714)

Narrow Bisect info : 

https://chromium.googlesource.com/chromium/src/+log/708062c2067e5611e1521879fc8c5748a3721fe5..e931d2aecb4d72946962e57e7e1586668071b504

Suspecting: https://chromium.googlesource.com/chromium/src/+/e931d2aecb4d72946962e57e7e1586668071b504

@Philip Rogers: Could you please help to reassign if your change is not the cause for this change.
Note:
1.Error shown while doing with new-script(RuntimeError: We don't have enough builds to bisect. revlist: [])
2.Unable to provide bisect using per-revision script,Hence providing bisect with old script.
3.Issue is not seen on  Windows(7,8,8.1,10) and Linux(14.04 LTS).


Thank You!
 
Actual_Result.mov
9.3 MB View Download
Expected_Result.mov
10.9 MB View Download
Cc: ellyjo...@chromium.org

Comment 2 by pdr@chromium.org, May 2 2018

Here's a minimized repro:
<!doctype html>
<div style="position: fixed;">
  <div style="background: blue; width: 30px; height: 30px;"></div>
  <div style="transform: translateY(-4500px); width: 100px; height: 100px;"></div>
</div>
<div id="forcescroll" style="height: 2000px;"></div>

Cc: chrishtr@chromium.org susan.boorgula@chromium.org
 Issue 837903  has been merged into this issue.
Is all well? need any help with this bug?

Comment 5 by pdr@chromium.org, May 7 2018

Cc: pdr@chromium.org
 Issue 840318  has been merged into this issue.
Labels: ReleaseBlock-Stable M-67
Cc: chrisha@chromium.org pmonette@chromium.org
 Issue 839482  has been merged into this issue.
Labels: Hotlist-ConOps
This looks like the same bug that's causing Facebook chat windows to disappear too, but let me know if I should file a new bug.
chat-box-rendering.JPG
54.5 KB View Download
chat-box-rendering2.JPG
17.6 KB View Download
Project Member

Comment 9 by bugdroid1@chromium.org, May 9 2018

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

commit cfebac612b10fdf961dca9df2bb28581577a612a
Author: Philip Rogers <pdr@chromium.org>
Date: Wed May 09 02:29:02 2018

[RLS] Ignore fixed when applying root scroll offset for interest rect

This patch fixes a regression from [1] where
MapToVisualRectInAncestorSpaceInternal was changed to not apply a
counterscroll offset for fixed position descendants. Interest rect
calculations incorrectly relied on this behavior.

MapToVisualRectInAncestorSpace is exclusive of the clip and scroll on
the ancestor object. To account for this, the interest rect logic would
call MapToVisualRectInAncestorSpace and then apply the root clip and
scroll offset, but this is not correct if there are fixed-position
children.

This patch updates the callsite to MapToVisualRectInAncestorSpace to
map to nullptr instead of the root view which will include the layout
view's clip and scroll.

[1] http://crrev.com/e931d2aecb

Bug:  838111 ,  831380 ,  840504 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I14b023279ef58e0123febae367a2ea0c67733c50
Reviewed-on: https://chromium-review.googlesource.com/1047193
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557066}
[modify] https://crrev.com/cfebac612b10fdf961dca9df2bb28581577a612a/third_party/blink/renderer/core/layout/layout_object.h
[modify] https://crrev.com/cfebac612b10fdf961dca9df2bb28581577a612a/third_party/blink/renderer/core/layout/layout_view.cc
[modify] https://crrev.com/cfebac612b10fdf961dca9df2bb28581577a612a/third_party/blink/renderer/core/layout/layout_view.h
[modify] https://crrev.com/cfebac612b10fdf961dca9df2bb28581577a612a/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
[modify] https://crrev.com/cfebac612b10fdf961dca9df2bb28581577a612a/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping_test.cc

Comment 10 by pdr@chromium.org, May 9 2018

 Issue 841194  has been merged into this issue.

Comment 11 by pdr@chromium.org, May 9 2018

Sadly, this missed yesterday's Canary. I will verify this fix in tomorrow's canary and request a merge into M67.
NextAction: 2018-05-10
The NextAction date has arrived: 2018-05-10

Comment 14 by siggi@chromium.org, May 10 2018

Went to try and verify, but unfortunately today's canary is DOA for me due to   https://crbug.com/841685 .

Comment 15 by pdr@chromium.org, May 10 2018

Cc: weiliangc@chromium.org ajuma@chromium.org
 Issue 840504  has been merged into this issue.

Comment 16 by pdr@chromium.org, May 10 2018

Labels: RegressedIn-67 Merge-Request-67
I verified the fix is in Canary and works.

Requesting a merge of the patch in Comment 9 to M67.
Labels: -Merge-Request-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comment #16. 
Please merge ASAP and mark bug as fixed after the merge if nothing else is pending. Thank you.
Project Member

Comment 18 by bugdroid1@chromium.org, May 10 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/657835337eb4fc280366c4ddbcc987a1b8db431b

commit 657835337eb4fc280366c4ddbcc987a1b8db431b
Author: Philip Rogers <pdr@chromium.org>
Date: Thu May 10 16:29:23 2018

[RLS] Ignore fixed when applying root scroll offset for interest rect

This patch fixes a regression from [1] where
MapToVisualRectInAncestorSpaceInternal was changed to not apply a
counterscroll offset for fixed position descendants. Interest rect
calculations incorrectly relied on this behavior.

MapToVisualRectInAncestorSpace is exclusive of the clip and scroll on
the ancestor object. To account for this, the interest rect logic would
call MapToVisualRectInAncestorSpace and then apply the root clip and
scroll offset, but this is not correct if there are fixed-position
children.

This patch updates the callsite to MapToVisualRectInAncestorSpace to
map to nullptr instead of the root view which will include the layout
view's clip and scroll.

[1] http://crrev.com/e931d2aecb

Bug:  838111 ,  831380 ,  840504 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I14b023279ef58e0123febae367a2ea0c67733c50
Reviewed-on: https://chromium-review.googlesource.com/1047193
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#557066}(cherry picked from commit cfebac612b10fdf961dca9df2bb28581577a612a)
Reviewed-on: https://chromium-review.googlesource.com/1054110
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#545}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/657835337eb4fc280366c4ddbcc987a1b8db431b/third_party/blink/renderer/core/layout/layout_object.h
[modify] https://crrev.com/657835337eb4fc280366c4ddbcc987a1b8db431b/third_party/blink/renderer/core/layout/layout_view.cc
[modify] https://crrev.com/657835337eb4fc280366c4ddbcc987a1b8db431b/third_party/blink/renderer/core/layout/layout_view.h
[modify] https://crrev.com/657835337eb4fc280366c4ddbcc987a1b8db431b/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
[modify] https://crrev.com/657835337eb4fc280366c4ddbcc987a1b8db431b/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping_test.cc

Comment 19 by pdr@chromium.org, May 10 2018

Status: Fixed (was: Assigned)

Comment 20 by grt@chromium.org, May 11 2018

Status: Assigned (was: Fixed)
The steps in  issue 841194  still repro for me in 68.0.3427.0 (Official Build) canary (64-bit) (cohort: Clang-64).

Comment 21 by pdr@chromium.org, May 11 2018

It looks like  issue 841194  is actually a different bug. I've de-duped and will continue working on that separately.
Can this be marked as fix now as per comment #21?

Comment 23 by pdr@chromium.org, May 11 2018

Status: Fixed (was: Assigned)

Comment 24 by pdr@chromium.org, May 11 2018

Issue 840738 has been merged into this issue.
Labels: TE-Verified-M67 TE-Verified-67.0.3396.44
Update : 
Retested above issue on (10.12.6,10.13.1,10.13.5) OS using Beta #67.0.3396.44 and issue is fixed.'Live Ticker Matches'Bar is in proper postion and does not  scroll along with page .
Kindly review the attached screen-cast.

Thank you!

67.0.3396.44Behaviour.mov
5.7 MB View Download
Project Member

Comment 26 by bugdroid1@chromium.org, May 15 2018

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

commit 82613a3b2625b939526277ac44e2e3bd20cdd424
Author: Philip Rogers <pdr@chromium.org>
Date: Tue May 15 02:13:50 2018

Revert: [RLS] Ignore fixed when applying root scroll offset for interest rect

This patch reverts [1] because we have a more general solution in [2].
In addition, through manual code inspection, we found [1] has a bug for
OOPIF because mapping to nullptr goes through remote frames whereas we
only intend to map to the local root. The test from [1] has not been
reverted.

[1] https://chromium.googlesource.com/chromium/src/+/cfebac612b10fdf961dca9df2bb28581577a612a
[2] https://chromium.googlesource.com/chromium/src/+/edc066c316b24fb3dea226ae6c1c5c1fdee062c3

Bug:  838111 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I0810c0eacb492ac413d31c041e97fce6ad23932b
Reviewed-on: https://chromium-review.googlesource.com/1058673
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558587}
[modify] https://crrev.com/82613a3b2625b939526277ac44e2e3bd20cdd424/third_party/blink/renderer/core/layout/layout_view.cc
[modify] https://crrev.com/82613a3b2625b939526277ac44e2e3bd20cdd424/third_party/blink/renderer/core/layout/layout_view.h
[modify] https://crrev.com/82613a3b2625b939526277ac44e2e3bd20cdd424/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc

CL listed at #26 is also merged to M67 branch 3396 at #18. Is this need to be reverted from M67 as well?

Comment 28 by pdr@chromium.org, May 15 2018

Status: Assigned (was: Fixed)
Yes, we need to merge this revert once https://bugs.chromium.org/p/chromium/issues/detail?id=841194 is merged.

Comment 29 by pdr@chromium.org, May 15 2018

Cc: abdulsyed@chromium.org ligim...@chromium.org jmukthavaram@chromium.org pbomm...@chromium.org gov...@chromium.org ajha@chromium.org
 Issue 838093  has been merged into this issue.
Re #28, M67 merge is done for: https://bugs.chromium.org/p/chromium/issues/detail?id=841194#c19 

Comment 31 by pdr@chromium.org, May 15 2018

Labels: Merge-Request-67
Project Member

Comment 32 by sheriffbot@chromium.org, May 15 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-67 Merge-Approved-67
Per offline chat with pdr@, this is a safe revert merge. It's reverting a partial solution but we have merged a better fix to the fundamental bug. So we can revert the partial solution now.

Approving merge to M67 branch 3396. Pls merge ASAP. Thank you.

Comment 34 by pdr@chromium.org, May 15 2018

Adding a bit of info for posterity: The original patch (Comment 9, https://crrev.com/cfebac612b) in this bug was just a partial solution. We have fixed a more fundamental bug in  https://crbug.com/841194 , https://crrev.com/edc066c31 and can revert https://crrev.com/cfebac612b.
Project Member

Comment 35 by bugdroid1@chromium.org, May 15 2018

Labels: -merge-approved-67
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a124290ed0623558ae0a30354cbf401ef0654669

commit a124290ed0623558ae0a30354cbf401ef0654669
Author: Philip Rogers <pdr@chromium.org>
Date: Tue May 15 20:19:59 2018

Revert: [RLS] Ignore fixed when applying root scroll offset for interest rect

This patch reverts [1] because we have a more general solution in [2].
In addition, through manual code inspection, we found [1] has a bug for
OOPIF because mapping to nullptr goes through remote frames whereas we
only intend to map to the local root. The test from [1] has not been
reverted.

[1] https://chromium.googlesource.com/chromium/src/+/cfebac612b10fdf961dca9df2bb28581577a612a
[2] https://chromium.googlesource.com/chromium/src/+/edc066c316b24fb3dea226ae6c1c5c1fdee062c3

TBR=pdr@chromium.org

(cherry picked from commit 82613a3b2625b939526277ac44e2e3bd20cdd424)

Bug:  838111 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I0810c0eacb492ac413d31c041e97fce6ad23932b
Reviewed-on: https://chromium-review.googlesource.com/1058673
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#558587}
Reviewed-on: https://chromium-review.googlesource.com/1060354
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#607}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/a124290ed0623558ae0a30354cbf401ef0654669/third_party/blink/renderer/core/layout/layout_view.cc
[modify] https://crrev.com/a124290ed0623558ae0a30354cbf401ef0654669/third_party/blink/renderer/core/layout/layout_view.h
[modify] https://crrev.com/a124290ed0623558ae0a30354cbf401ef0654669/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc

Comment 36 by pdr@chromium.org, May 15 2018

Status: Fixed (was: Assigned)
Labels: TE-Verified-68.0.3432.0 TE-Verified-M68
Update : 
Retested above issue on (10.12.6,10.13.1,10.13.5) OS using Canary #68.0.3432.0 and issue is fixed.'Live Ticker Matches'Bar is in proper position and does not  scroll along with page .
Kindly review the attached screen-cast.

Thank you!

Canary_Behaviour68.0.3432.0.mov
5.0 MB View Download

Sign in to add a comment