New issue
Advanced search Search tips

Issue 841194 link

Starred by 6 users

Issue metadata

Status: Fixed
Merged: issue 838111
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-15
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Hangouts mole goes transparent in Inbox

Project Member Reported by grt@chromium.org, May 9 2018

Issue description

You are probably looking for a change made after 67.0.3396.20 (known good), but no later than 67.0.3396.22 (first known bad).
CHANGELOG URL:
https://chromium.googlesource.com/chromium/src/+log/67.0.3396.20..67.0.3396.22?pretty=full

Repro steps:
- https://inbox.google.com/
- click the hangouts icon in the blue bar at the top of the page
- click on a conversation to open the chat mole
- scroll down in the inbox

The contents of the chat mole go away/transparent leaving behind the drop shadow. The mole continues to respond to mouse events, so I believe it's a display issue.
 

Comment 1 by grt@chromium.org, May 9 2018

Components: -Blink Blink>Scroll
Owner: pdr@chromium.org
Status: Assigned (was: Untriaged)
You are probably looking for a change made after 553449 (known good), but no later than 553456 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/956820a1235d55d1d7f0c1df1fbfb9034934ca7d..cde65892e73020b11c58cd99f7e218d0826246c8

I'm guessing r553455: [RLS] Ignore fixed when applying root scroll offset for interest rect

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

Mergedinto: 838111
Status: Duplicate (was: Assigned)
Thank you for taking the time to bisect and file this. It should be fixed in Chrome Canary and I will merge this fix as well.

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

Correction: this should be fixed in tomorrow's Chrome Canary.

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

Status: Assigned (was: Duplicate)
I tested today and am still able to reproduce in 68.0.3427.0 which should have the fix. De-duping.
I was not able to reproduce. (https://bugs.chromium.org/p/chromium/issues/detail?id=839482 is the same bug as this one)

Comment 6 by siggi@chromium.org, May 11 2018

Cc: chrishtr@chromium.org chrisha@chromium.org pmonette@chromium.org
 Issue 839482  has been merged into this issue.

Comment 7 by siggi@chromium.org, May 11 2018

I still repro this on "Version 68.0.3427.0 (Official Build) canary (64-bit)" on my home machine.

Comment 8 by gov...@chromium.org, May 11 2018

Labels: M-68

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

Owner: chrishtr@chromium.org
I can also reproduce. Chris is going to take a look at this.
The bug is that scroll should be applied if ancestor == container. This broke
an intersection observer use-case for scrolled sub-frames, leading to them think they were throttled.
Project Member

Comment 11 by bugdroid1@chromium.org, May 14 2018

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

commit edc066c316b24fb3dea226ae6c1c5c1fdee062c3
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon May 14 21:41:27 2018

Apply fixed-position scroll offset if the containing block is ancestor.

An earlier patch (commit e931d2aecb4d72946962e57e7e1586668071b504)
changed code in MapToVisualRectInAncestorSpace to omit fixed-position
scroll in cases where the containing block was the LayoutView and
a counter-scroll would have been applied in the LayoutView anyway.
This fixed a bug in which a clip was applied in-between in the
wrong space.

The CL introduced a bug, however, in the case when the LayoutView
was the desired ancestor passed to MapToVisualRectInAncestorSpace.
In this case we should apply the counter scroll in LayoutView, because
the results of MapToVisualRectInAncestorSpace are in the unscrolled
space of the LayoutView.

Note that
LayoutBox::MapToVisualRectInAncestorSpace already has code to
skip fixed-position scroll and clip if container == ancestor,
independent of the above commit.

Bug:841194

Change-Id: Ia05894dd94bdb3b0503cbb155728b0ef074b0227
Reviewed-on: https://chromium-review.googlesource.com/1058039
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558473}
[modify] https://crrev.com/edc066c316b24fb3dea226ae6c1c5c1fdee062c3/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/edc066c316b24fb3dea226ae6c1c5c1fdee062c3/third_party/blink/renderer/core/layout/visual_rect_mapping_test.cc

NextAction: 2018-05-15
Thank you Chris. Pls request a merge to M67 tomorrow if change looks good in canary so we can take it in for Beta release on Wednesday.
Able to reproduce the issue on build without fix(68.0.3430.0) using Windows 10, Mac 10.13.3 and Ubuntu 14.04. Hence verifying the fix on latest canary 68.0.3431.0.
Note: Verified the fix on Mac and Ubuntu and Unable to verify the same on Windows 32 & 64 due to issue: 843012.

Now no transparent hangouts chat window is seen on scrolling the page. Hence fix is working as expected(...on Mac and Ubuntu)

Thanks!
841194_68.0.3431.0.mp4
5.9 MB View Download
The NextAction date has arrived: 2018-05-15
Labels: Merge-Request-67
Verifed fixed in 68.0.3431.0 / Mac w/
--disable-prefer-compositing-to-lcd-text
Labels: OS-Android OS-Chrome OS-Linux OS-Mac
Project Member

Comment 17 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
Approving merge to M67 branch 3396 based on comment #13 and #15. Pls merge ASAP. Thank you.
Project Member

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

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

commit 3d99c213a6eddafa7e8a0e7aba7e10952b8d5620
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Tue May 15 17:29:19 2018

Apply fixed-position scroll offset if the containing block is ancestor.

An earlier patch (commit e931d2aecb4d72946962e57e7e1586668071b504)
changed code in MapToVisualRectInAncestorSpace to omit fixed-position
scroll in cases where the containing block was the LayoutView and
a counter-scroll would have been applied in the LayoutView anyway.
This fixed a bug in which a clip was applied in-between in the
wrong space.

The CL introduced a bug, however, in the case when the LayoutView
was the desired ancestor passed to MapToVisualRectInAncestorSpace.
In this case we should apply the counter scroll in LayoutView, because
the results of MapToVisualRectInAncestorSpace are in the unscrolled
space of the LayoutView.

Note that
LayoutBox::MapToVisualRectInAncestorSpace already has code to
skip fixed-position scroll and clip if container == ancestor,
independent of the above commit.

Bug:841194

TBR=chrishtr@chromium.org

(cherry picked from commit edc066c316b24fb3dea226ae6c1c5c1fdee062c3)

Change-Id: Ia05894dd94bdb3b0503cbb155728b0ef074b0227
Reviewed-on: https://chromium-review.googlesource.com/1058039
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#558473}
Reviewed-on: https://chromium-review.googlesource.com/1060030
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#604}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/3d99c213a6eddafa7e8a0e7aba7e10952b8d5620/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/3d99c213a6eddafa7e8a0e7aba7e10952b8d5620/third_party/blink/renderer/core/layout/visual_rect_mapping_test.cc

Status: Fixed (was: Assigned)
Android: Issue verified on 67.0.3396.49 and 68.0.3432.0

Comment 22 by grt@chromium.org, May 16 2018

Looks good to me on Windows in 68.0.3432.1 (Official Build) canary-dcheck (32-bit) (cohort: ASAN)

Comment 23 by siggi@chromium.org, May 16 2018

Looks good here too on Windows Version 68.0.3432.0 (Official Build) canary (64-bit)

Comment 24 by bokan@chromium.org, May 24 2018

Cc: bokan@chromium.org vamshi.kommuri@chromium.org
 Issue 845861  has been merged into this issue.

Sign in to add a comment