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

Issue 831380 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

The wp media modal disappears at certain scroll levels in Chrome Beta

Reported by lukeshar...@gmail.com, Apr 10 2018

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0

Steps to reproduce the problem:
1. http://demo.sharefaithwebsites.com/template69
2. Start a demo signing up with any email address.
3. Click within text to edit, wait for toolbar.
4. Click "Media" on the black toolbar.

What is the expected behavior?
The media modal should be visible, as in Chrome stable.

What went wrong?
It only shows the grey overlay over the page, no dialog... unless I scroll up and down the page, then it hides or shows with no apparent DOM change.

Did this work before? Yes Version 65.0.3325.181 (Official Build) (64-bit)

Chrome version: Version 66.0.3359.81 (Official Build) beta (64-bit)  Channel: beta
OS Version: Ubuntu 16.04
Flash Version: Shockwave Flash 29.0 r0
 
Labels: Needs-Bisect Needs-Triage-M66
Cc: vamshi.kommuri@chromium.org
Labels: Triaged-ET Needs-Feedback
Thanks for filing the issue!

Unable to reproduce the issue on reported chrome version 66.0.3359.81 and on the latest 67.0.3386.1 using Ubuntu 14.04 and Ubuntu 17.10 with the below mentioned steps.
1. Launched chrome
2. Navigated to http://demo.sharefaithwebsites.com/template69
3. Singed in and clicked on text to edit
4. After toolbar appears clicked on Media
We observed a dialogue box appearing. Similar behaviour is seen in chrome stable 65.0.3325.181. Attaching the screen cast of the same.

@Reporter: Could you please have a look at the screen cast and let us know if we have missed anything in the process. Any further inputs from your end may help us.
831380.mp4
4.9 MB View Download
I see that too, that media popup apparently will only show up when you are scrolled far to the top in the area you showed... if you had clicked edit on any section you need to scroll down to, then click the media button, it will not show the modal unless you scroll around, it shows the dialog suddenly if you scroll to the top of the screen.

Just tested and I'm seeing the issue on another HiDPI computer here too.
Project Member

Comment 4 by sheriffbot@chromium.org, Apr 11 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Needs-Feedback
Tried checking the issue as per comment#4 on reported chrome version 66.0.3359.81 using Ubuntu 14.04 with the below mentioned steps.
1. After signing into the given URL
2. Clicked on text to edit
3. Scrolled the page, then clicked on media in toolbar.
We observed pop-up(modal) just after clicking the media without being scrolling the page. Attaching the screen cast of the same.

@Reporter: Could you please have a look at the screen cast and let us know if anything missed from our end. Providing with a screen cast explaining the issue would help us in understanding the issue in a better way.

Thanks!
831380.mp4
4.6 MB View Download
You're still editing only the very top post, if you scrolled down and clicked to edit anywhere in the middle or bottom part of that page the media modal only shows up and works when you scroll up to the top of the page, only on Chrome-beta. Thanks for checking again.
Project Member

Comment 7 by sheriffbot@chromium.org, Apr 12 2018

Labels: -Needs-Feedback
Thank you for providing more feedback. Adding the requester to the cc list.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Pri-2 -Needs-Bisect hasbisect-per-revision Target-67 Target-66 M-66 FoundIn-66 FoundIn-67 ReleaseBlock-Stable RegressedIn-66 OS-Mac OS-Windows Pri-1
Owner: bokan@chromium.org
Status: Assigned (was: Unconfirmed)
Checked the issue as per comment#6.

Able to reproduce the issue on reported chrome version 66.0.3359.81 and on the latest canary 67.0.3395.0 using Windows 10, Ubuntu 14.04 and Mac 10.13.1.

Bisect Information:
===================
Last Good Build: 66.0.3347.0
First Bad Build: 66.0.3348.0

You are probably looking for a change made after 536891 (known good), but no later than 536892 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/296e89cb3e93e39b3caa810bc3df1d67e5de4e0e..39cb84e649e045d2233a395009d5ccf1d08854a0

Suspecting: https://chromium.googlesource.com/chromium/src/+/39cb84e649e045d2233a395009d5ccf1d08854a0
Review URL: https://chromium-review.googlesource.com/919644

@David Bokan: Please help us in assigning it to the right owner, if this isn't related to your change.

Adding RB-Stable as this seems to be a recent regression.

Thanks!


Comment 9 by bokan@chromium.org, Apr 13 2018

Cc: bokan@chromium.org chrishtr@chromium.org
Components: -UI Blink>Scroll
Owner: pdr@chromium.org
The page is rather complex but I managed to boil it down to a minimized test case. The layer tree doesn't change so this appears to be a paint issue. pdr@ mind taking it from here?
831380.html
425 bytes View Download
I don't think this is fully release blocking stable. We should address this for M67. Pdr@ do you agree?
Labels: -ReleaseBlock-Stable

Comment 12 by pdr@chromium.org, Apr 17 2018

Reporter: Thank you for taking the time to file this and followup with help to reproduce the issue. Because we are so close to Chrome 66 releasing and this is somewhat difficult to hit, I think we will unfortunately release without fixing this bug. I will try to get this fixed ASAP for Chrome 67.
So the only workaround will be to scroll up? Looks like it's not taking to the usual change to margin/top to get Chrome to rerender that usually works with repainting bugs like this.

Comment 14 by bokan@chromium.org, Apr 17 2018

FWIW, I narrowed the issue down to the <span class='screen-reader-text'> inside the dialog's close button - if you remove the `left: -1000em` (and find some other way to hide it) that removes the bug.
Thanks for the workaround. FYI this is probably the same underlying issue as this longstanding issue too: https://bugs.chromium.org/p/chromium/issues/detail?id=800638

Comment 16 by bokan@chromium.org, Apr 24 2018

Cc: phanindra.mandapaka@chromium.org
 Issue 834239  has been merged into this issue.
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 25 2018

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

commit e931d2aecb4d72946962e57e7e1586668071b504
Author: Philip Rogers <pdr@chromium.org>
Date: Wed Apr 25 05:13:01 2018

[RLS] Do not apply scroll when clipping fixed-position visual rects

With root layer scrolling the layout view clips and we need to ensure
the clip occurs in the correct (un)scrolled space for fixed position.
This patch changes the fixed position case: we stop applying the layout
view scroll, clip, and then do not counter-scroll. There is a similar
codepath for the !RLS case which has not been changed in this patch.

Here's a summarized description of where scroll offsets apply:
1) MapToVisualRectInAncestorSpaceInternal calls MapVisualRectToContainer
2) MapVisualRectToContainer calls MapContentsRectToBoxSpace
3) MapContentsRectToBoxSpace applies the layout view scroll offset
4) MapContentsRectToBoxSpace clips
5) MapToVisualRectInAncestorSpaceInternal un-applies the scroll offset

This patch removes #3 and #5 for fixed position.

Bug:  831380 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I3437d3933155960abfcbebd437ddb6cb606ea37e
Reviewed-on: https://chromium-review.googlesource.com/1025329
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553455}
[modify] https://crrev.com/e931d2aecb4d72946962e57e7e1586668071b504/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/e931d2aecb4d72946962e57e7e1586668071b504/third_party/blink/renderer/core/layout/layout_box.h
[modify] https://crrev.com/e931d2aecb4d72946962e57e7e1586668071b504/third_party/blink/renderer/core/layout/layout_inline.cc
[modify] https://crrev.com/e931d2aecb4d72946962e57e7e1586668071b504/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/e931d2aecb4d72946962e57e7e1586668071b504/third_party/blink/renderer/core/layout/visual_rect_mapping_test.cc
[modify] https://crrev.com/e931d2aecb4d72946962e57e7e1586668071b504/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping_test.cc

Comment 18 by pdr@chromium.org, Apr 25 2018

Labels: Merge-Request-67 OS-Android OS-Chrome
I'd like to let this bake for a few days, then merge into the 67 pie.
Project Member

Comment 19 by sheriffbot@chromium.org, Apr 26 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact 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
Pls merge your change to M67 branch 3396 ASAP so we can pick it up for next M67 Dev/Beta release. Thank you.
Project Member

Comment 21 by bugdroid1@chromium.org, Apr 26 2018

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

commit 60e949469127b8df5e94572448db96758b34668f
Author: Philip Rogers <pdr@chromium.org>
Date: Thu Apr 26 18:09:53 2018

[RLS] Do not apply scroll when clipping fixed-position visual rects

With root layer scrolling the layout view clips and we need to ensure
the clip occurs in the correct (un)scrolled space for fixed position.
This patch changes the fixed position case: we stop applying the layout
view scroll, clip, and then do not counter-scroll. There is a similar
codepath for the !RLS case which has not been changed in this patch.

Here's a summarized description of where scroll offsets apply:
1) MapToVisualRectInAncestorSpaceInternal calls MapVisualRectToContainer
2) MapVisualRectToContainer calls MapContentsRectToBoxSpace
3) MapContentsRectToBoxSpace applies the layout view scroll offset
4) MapContentsRectToBoxSpace clips
5) MapToVisualRectInAncestorSpaceInternal un-applies the scroll offset

This patch removes #3 and #5 for fixed position.

Bug:  831380 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I3437d3933155960abfcbebd437ddb6cb606ea37e
Reviewed-on: https://chromium-review.googlesource.com/1025329
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#553455}(cherry picked from commit e931d2aecb4d72946962e57e7e1586668071b504)
Reviewed-on: https://chromium-review.googlesource.com/1030871
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#333}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/60e949469127b8df5e94572448db96758b34668f/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/60e949469127b8df5e94572448db96758b34668f/third_party/blink/renderer/core/layout/layout_box.h
[modify] https://crrev.com/60e949469127b8df5e94572448db96758b34668f/third_party/blink/renderer/core/layout/layout_inline.cc
[modify] https://crrev.com/60e949469127b8df5e94572448db96758b34668f/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/60e949469127b8df5e94572448db96758b34668f/third_party/blink/renderer/core/layout/visual_rect_mapping_test.cc
[modify] https://crrev.com/60e949469127b8df5e94572448db96758b34668f/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping_test.cc

Comment 22 by pdr@chromium.org, Apr 26 2018

Status: Fixed (was: Assigned)
Project Member

Comment 23 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 24 by pdr@chromium.org, May 10 2018

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

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

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 26 by kochi@chromium.org, May 24 2018

Cc: kochi@chromium.org
 Issue 846231  has been merged into this issue.
 Issue 848176  has been merged into this issue.

Sign in to add a comment