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

foreignObject hit test offset is wrong

Reported by batist.l...@gmail.com, May 31 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.62 Safari/537.36

Steps to reproduce the problem:
1. Open: https://jsbin.com/qozapirone/edit?html,output (or the attached file)
2. Right click the text element

What is the expected behavior?
The context menu should open on the position of the cursor.

What went wrong?
The context menu does not open on the correct position.

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

Does this work in other browsers? N/A

Chrome version: 67.0.3396.62  Channel: stable
OS Version: OS X 10.13.4
Flash Version: 

I think this might result in other problems, such as click event not registering correctly in some cases. (investigating)
 
foreignObjectTest.html
537 bytes View Download

Comment 1 by gov...@chromium.org, May 31 2018

Cc: pbomm...@chromium.org
Labels: Needs-Triage-M67
Components: Blink>SVG
Labels: Needs-Bisect
I can confirm that this causes other issues with mouse events.
Only mousedown seems to work correctly.
It worked on Chromium 67.0.3362.0 but not on Chrome 67.0.3396.62.

There is also some cases where foreign object contents disappears or elements with pointer-event set to none are clicked anyway. I assume it's all related but I can open others issues if necessary.

The attached file is SVG-edit with a slightly modified foreign object extension that adds console log for mousedown, mouseup, click and dblclick. You can add foreign objects using the last button from the menu on the left. Take note that the misplaced selector handles is not new, there is a workaround not included in this setup. This example must be hosted somewhere to be accessible using HTTP, SVG-edit does not work correctly using local files.
svg-edit-chrome-issues.zip
842 KB Download
Labels: -Pri-2 RegressedIn-67 Target-67 FoundIn-67 FoundIn-68 Pri-1
Owner: chrishtr@chromium.org
Status: Assigned (was: Unconfirmed)
This seems to be Mac only. Linux works fine for me.

Bisects to https://chromium.googlesource.com/chromium/src/+/c183d574e98671f43fab8445c6904d28065a64a5

Which was merged back to M-67 in https://chromium.googlesource.com/chromium/src/+/c6a11b77536d22de7966dd23d06973feb53f8ca4

So this is a stable regression.
I have users reporting this problem using Windows 7.
Labels: OS-Windows
Thanks for the Windows 7 report.
Components: Blink>HitTesting
Summary: foreignObject hit test offset is wrong (was: foreignObject offset is wrong)
batist.leman@ and sdeblois@, we are assessing the impact of this bug in order to decide how to proceed. Can you tell us the extent to which this affected user-facing content? Does it make it impossible to use a site?
Our CMS offers a visual editor that relies heavily on foreign objects.
The mouse events issue makes it unusable on Chrome.
We are telling people to use Firefox until this is fixed.
For us as well, we have a visual editor that uses the foreignObject to handle text fields. We also advise to use Firefox at the moment.
Labels: ReleaseBlock-Stable M-68 Target-68

Comment 11 Deleted

Adding All OSs except iOS as this is a Blink bug. Pls feel free to remove if not applicable.

We're halting M67 further roll out for Desktop due to this bug per offline discussion with chrishtr@. 
Labels: M-67
Labels: -Needs-Bisect -Needs-Triage-M67
Labels: M-69 Target-69
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 1 2018

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

commit 427f64a95fe021629f0fb51642ba27852d4411a8
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Fri Jun 01 03:24:31 2018

[foreignObject] Fix hit testing of positioned, clipped descendants of <foreignObject>

The code was broken in three ways:

1. Failing to not re-apply LayoutBoxLocation in PaintLayer. This caused the cull rect
check in LayoutBox::HitTestAllPhases to fail because the coordinate space of the
accumulated offset was wrong by 100px.

2. Coordinate space of the hit test location was wrong in callsite in LayoutSVGForeignObject.
This resulted in culling in PaintLayer::HitTestContentsForFragments early outing because
of lack of intersection with the foreground/background clip rect.

#2 is why overflow:hidden is required to reproduce the full problem, because otherwise the clip
rect will be infinite and therefore it doesn't matter what the coordinate space was.

3. Transforms of <foreignObject> were applied twice: once in LayoutSVGForeignObject, and once
in PaintLayer. Now we apply it in LayoutSVGForeignObject only.

Bug:848179

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I335416a267bfa9a9df2dc600231fe1518439e790
Reviewed-on: https://chromium-review.googlesource.com/1081241
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563518}
[modify] https://crrev.com/427f64a95fe021629f0fb51642ba27852d4411a8/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc
[modify] https://crrev.com/427f64a95fe021629f0fb51642ba27852d4411a8/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object_test.cc
[modify] https://crrev.com/427f64a95fe021629f0fb51642ba27852d4411a8/third_party/blink/renderer/core/paint/paint_layer.cc

Labels: Merge-Request-68 Merge-Request-67
Project Member

Comment 18 by sheriffbot@chromium.org, Jun 1 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
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
Project Member

Comment 19 by bugdroid1@chromium.org, Jun 1 2018

Labels: merge-merged-3447
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc0a58238ac790abe7e55416df9f4b3001712076

commit cc0a58238ac790abe7e55416df9f4b3001712076
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Fri Jun 01 05:08:11 2018

[foreignObject] Fix hit testing of positioned, clipped descendants of <foreignObject>

The code was broken in three ways:

1. Failing to not re-apply LayoutBoxLocation in PaintLayer. This caused the cull rect
check in LayoutBox::HitTestAllPhases to fail because the coordinate space of the
accumulated offset was wrong by 100px.

2. Coordinate space of the hit test location was wrong in callsite in LayoutSVGForeignObject.
This resulted in culling in PaintLayer::HitTestContentsForFragments early outing because
of lack of intersection with the foreground/background clip rect.

rect will be infinite and therefore it doesn't matter what the coordinate space was.

3. Transforms of <foreignObject> were applied twice: once in LayoutSVGForeignObject, and once
in PaintLayer. Now we apply it in LayoutSVGForeignObject only.

Bug:848179

TBR=chrishtr@chromium.org

(cherry picked from commit 427f64a95fe021629f0fb51642ba27852d4411a8)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I335416a267bfa9a9df2dc600231fe1518439e790
Reviewed-on: https://chromium-review.googlesource.com/1081241
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563518}
Reviewed-on: https://chromium-review.googlesource.com/1082027
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3447@{#3}
Cr-Branched-From: b8db51736cc3a3bda2a15aa8805871f9f12c0853-refs/heads/master@{#563478}
[modify] https://crrev.com/cc0a58238ac790abe7e55416df9f4b3001712076/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc
[modify] https://crrev.com/cc0a58238ac790abe7e55416df9f4b3001712076/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object_test.cc
[modify] https://crrev.com/cc0a58238ac790abe7e55416df9f4b3001712076/third_party/blink/renderer/core/paint/paint_layer.cc

Cc: ajha@chromium.org
Thank you chrishtr@ for merging to canary branch 3447. I'm triggering new canary from same branch.

New canary #69.0.3447.2 in progress which includes merge listed at #19.

ajha@, pls test/verify it on canary version #69.0.3447.2. Thank you.
Cc: susan.boorgula@chromium.org
 Issue 848539  has been merged into this issue.

Comment 23 by ajha@chromium.org, Jun 1 2018

Labels: Needs-Feedback
Issue seems to be still reproducible on the latest canary with the merge from C#19 i.e. 69.0.3447.2 on Windows-10, Mac OS 10.13.3 and Linux Ubuntu 14.04 as per the test steps from C#0. The context menu doesn't open from position of the cursor. Attached screenshot of the same.

However the test case from the duped issue in C#22 i.e.  Issue 848539  is working fine on the latest canary 69.0.3447.2 as per the testing performed on Windows-10, Mac OS 10.13.3 and Linux Ubuntu 14.04.

chrishtr@: Could you please take a look at this as per the test case in C#0.

Thank you!
848179_postfix_m69_canary.png
154 KB View Download
The remaining bug is that PointInInnerNodeFrame is not being
propagated back properly into the ancestor HitTestResult struct.
Almost done with bugfix CL.
The original bug broke the dragging, hover tooltip and right click on my d3 visualization. I just tested on head and can confirm that the first two are already fixed, and hopefully that final change solves the right click too.

Will this change be cherry picked to an upcoming 67?
Yes, the plan is to merge fixes to all the issues to 67. (and 68)
Project Member

Comment 28 by bugdroid1@chromium.org, Jun 1 2018

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

commit 7ab0b6e91a28e881354ab5a440537ee3c198af9a
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Fri Jun 01 23:28:44 2018

[ForeignObject] Preserve PointInInnerNodeFrame across foreignObject hit tests.

This regressed in a recent patch, which created a new HitTestResult.

Bug:  848179 
Change-Id: I976f6d9742ca2f785e78ed56a09c77f76dc9e271
Reviewed-on: https://chromium-review.googlesource.com/1082987
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563859}
[modify] https://crrev.com/7ab0b6e91a28e881354ab5a440537ee3c198af9a/third_party/blink/renderer/core/layout/hit_test_result.h
[modify] https://crrev.com/7ab0b6e91a28e881354ab5a440537ee3c198af9a/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc
[modify] https://crrev.com/7ab0b6e91a28e881354ab5a440537ee3c198af9a/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object_test.cc

NextAction: 2018-06-04
Thank you chrishtr@ for landing change to trunk. Pls update the bug with canary result on Monday morning.

ajha@, pls test/verify this on latest canary (69.0.3448.0 or higher). Thank you.
Can anyone try to repo this issue on Android?

Comment 31 by aluo@chromium.org, Jun 2 2018

This right-click menu is not available in Android, so can't repro it.  chrishtr@, do you know if this issue can be triggered on Android?
Project Member

Comment 32 by sheriffbot@chromium.org, Jun 2 2018

Labels: -Merge-Request-68 Hotlist-Merge-Approved Merge-Approved-68
Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 33 by bugdroid1@chromium.org, Jun 2 2018

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

commit 655b065ad3defe832814be1953de5a480417c052
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Sat Jun 02 18:23:05 2018

[ForeignObject] Preserve PointInInnerNodeFrame across foreignObject hit tests.

This regressed in a recent patch, which created a new HitTestResult.

TBR=chrishtr@chromium.org

(cherry picked from commit 7ab0b6e91a28e881354ab5a440537ee3c198af9a)

Bug:  848179 
Change-Id: I976f6d9742ca2f785e78ed56a09c77f76dc9e271
Reviewed-on: https://chromium-review.googlesource.com/1082987
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563859}
Reviewed-on: https://chromium-review.googlesource.com/1084152
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3447@{#8}
Cr-Branched-From: b8db51736cc3a3bda2a15aa8805871f9f12c0853-refs/heads/master@{#563478}
[modify] https://crrev.com/655b065ad3defe832814be1953de5a480417c052/third_party/blink/renderer/core/layout/hit_test_result.h
[modify] https://crrev.com/655b065ad3defe832814be1953de5a480417c052/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc
[modify] https://crrev.com/655b065ad3defe832814be1953de5a480417c052/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object_test.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Jun 2 2018

Labels: -merge-approved-68 merge-merged-3440
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5be15367ec704e31efaf8c04ba067adbae79c075

commit 5be15367ec704e31efaf8c04ba067adbae79c075
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Sat Jun 02 18:25:36 2018

[foreignObject] Fix hit testing of positioned, clipped descendants of <foreignObject>

The code was broken in three ways:

1. Failing to not re-apply LayoutBoxLocation in PaintLayer. This caused the cull rect
check in LayoutBox::HitTestAllPhases to fail because the coordinate space of the
accumulated offset was wrong by 100px.

2. Coordinate space of the hit test location was wrong in callsite in LayoutSVGForeignObject.
This resulted in culling in PaintLayer::HitTestContentsForFragments early outing because
of lack of intersection with the foreground/background clip rect.

rect will be infinite and therefore it doesn't matter what the coordinate space was.

3. Transforms of <foreignObject> were applied twice: once in LayoutSVGForeignObject, and once
in PaintLayer. Now we apply it in LayoutSVGForeignObject only.

Bug:848179

TBR=chrishtr@chromium.org

(cherry picked from commit 427f64a95fe021629f0fb51642ba27852d4411a8)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I335416a267bfa9a9df2dc600231fe1518439e790
Reviewed-on: https://chromium-review.googlesource.com/1081241
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563518}
Reviewed-on: https://chromium-review.googlesource.com/1083963
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#118}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/5be15367ec704e31efaf8c04ba067adbae79c075/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc
[modify] https://crrev.com/5be15367ec704e31efaf8c04ba067adbae79c075/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object_test.cc
[modify] https://crrev.com/5be15367ec704e31efaf8c04ba067adbae79c075/third_party/blink/renderer/core/paint/paint_layer.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Jun 2 2018

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

commit 4855f3db94dcf23f31bcc1a998221fc40954a50f
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Sat Jun 02 18:27:11 2018

[ForeignObject] Preserve PointInInnerNodeFrame across foreignObject hit tests.

This regressed in a recent patch, which created a new HitTestResult.

TBR=chrishtr@chromium.org

(cherry picked from commit 7ab0b6e91a28e881354ab5a440537ee3c198af9a)

Bug:  848179 
Change-Id: I976f6d9742ca2f785e78ed56a09c77f76dc9e271
Reviewed-on: https://chromium-review.googlesource.com/1082987
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563859}
Reviewed-on: https://chromium-review.googlesource.com/1084153
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#119}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/4855f3db94dcf23f31bcc1a998221fc40954a50f/third_party/blink/renderer/core/layout/hit_test_result.h
[modify] https://crrev.com/4855f3db94dcf23f31bcc1a998221fc40954a50f/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc
[modify] https://crrev.com/4855f3db94dcf23f31bcc1a998221fc40954a50f/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object_test.cc

I have now merged both fixes into M68.

I also merged the second fix manually into canary branch 3447, since it seems
the original patch did not make the canary deadline.
Thank you chrishtr@ for merging to canary branch 3447, I've triggered new canary from same branch (69.0.3447.5). Pls user  69.0.3447.5 build to verify the fix.


FYI: Last night canary failed to trigger, pls see issue 848990.
Just tried on 69.0.3447.5, looks fixed to me.
Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comment #38. Pls merge on Monday morning if change continue to look good in canary. Thank you.
The NextAction date has arrived: 2018-06-04

Comment 41 by ajha@chromium.org, Jun 4 2018

Tested this on the latest canary: 69.0.3449.0 on Windows-10, Mac OS 10.13.3 and Linux Debian Rodete and below are the observations:

> Right click context menu of attached foreignObjectTest.html in C#0 seems to be working fine.

>https://jsfiddle.net/ss9j4ybv/3/ from  Issue 848539  duped here in C#22 seems to be working fine.

>https://jsbin.com/qozapirone/edit?html,output (C#0 jsbin) right click context menu seems to be opening fine but observing different issue as per this test case on the latest Mac canary: 69.0.3449.0, 10.13.3 Mac book Air(13.3 inch-1440*900).Same test case works fine on Mac Book Retina 10.13(15.4 inch-2880*1800).

Below are the repro steps:
-------------------------
1. Open https://jsbin.com/qozapirone/edit?html,output.
2. Right click on the text inside the textbox and open/close the context menu. Text gets selected.
3. Scroll down a bit and then scroll up and put the curson inside the text box and observe. Scroll up and down and observe.

Expected: Content should render properly.
Actual: Content inside the text box and somtimes textbox itself disappears on scroll up and down. Attaching the screen-cast of the same.

chrishtr@: Could you please take a look at this.

Thank you!
848179.webm
6.6 MB View Download

Thank you ajha@.
chrishtr@, ptal comment #41 before merging to M67. Thank you.
Looking into it.
Android: 
https://jsfiddle.net/ss9j4ybv/3/ from   Issue 848539   duped here in C#22 is working fine on latest 67.0.3396.74, 68.0.3440.14 and 69.0.3449.0
demo.mp4
1.9 MB View Download
The issue reported in #41 is yet another issue, unrelated to the other
ones. :( I have a fix for that as well.
Thank you  chrishtr@. 

Will the fix for #41 also need to merge to M67? 
I'm afraid, M67 is already in stable and we already have 2 fixes listed #16 & #28 to merge to M67. 

Yes, ideally that fix would also go into M67. I agree that this is a bad
situation. :(
Pls land the fix to trunk ASAP, I can trigger new canary with the merge if needed. Pls do make sure fix is fully safe to merge to Stable. Thank you.
Ok.

Note that the last fix that is not yet committed is really safe and will not
introduce any new bugs.
Project Member

Comment 51 by bugdroid1@chromium.org, Jun 4 2018

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

commit 04022115b93ed405fc89a6f9990ffc8189b31477
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon Jun 04 18:53:42 2018

[foreignObject] Fix hit testing of positioned, clipped descendants of <foreignObject>

The code was broken in three ways:

1. Failing to not re-apply LayoutBoxLocation in PaintLayer. This caused the cull rect
check in LayoutBox::HitTestAllPhases to fail because the coordinate space of the
accumulated offset was wrong by 100px.

2. Coordinate space of the hit test location was wrong in callsite in LayoutSVGForeignObject.
This resulted in culling in PaintLayer::HitTestContentsForFragments early outing because
of lack of intersection with the foreground/background clip rect.

rect will be infinite and therefore it doesn't matter what the coordinate space was.

3. Transforms of <foreignObject> were applied twice: once in LayoutSVGForeignObject, and once
in PaintLayer. Now we apply it in LayoutSVGForeignObject only.

Bug:848179

TBR=chrishtr@chromium.org

(cherry picked from commit 427f64a95fe021629f0fb51642ba27852d4411a8)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I335416a267bfa9a9df2dc600231fe1518439e790
Reviewed-on: https://chromium-review.googlesource.com/1081241
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563518}
Reviewed-on: https://chromium-review.googlesource.com/1085748
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#736}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/04022115b93ed405fc89a6f9990ffc8189b31477/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc
[modify] https://crrev.com/04022115b93ed405fc89a6f9990ffc8189b31477/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object_test.cc
[modify] https://crrev.com/04022115b93ed405fc89a6f9990ffc8189b31477/third_party/blink/renderer/core/paint/paint_layer.cc

Project Member

Comment 52 by bugdroid1@chromium.org, Jun 4 2018

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

commit f6ba195ee4592bf6f740ebc2060e927cc893b472
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon Jun 04 18:57:35 2018

[ForeignObject] Preserve PointInInnerNodeFrame across foreignObject hit tests.

This regressed in a recent patch, which created a new HitTestResult.

TBR=chrishtr@chromium.org

(cherry picked from commit 7ab0b6e91a28e881354ab5a440537ee3c198af9a)

Bug:  848179 
Change-Id: I976f6d9742ca2f785e78ed56a09c77f76dc9e271
Reviewed-on: https://chromium-review.googlesource.com/1082987
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Tien-Ren Chen <trchen@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#563859}
Reviewed-on: https://chromium-review.googlesource.com/1085807
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#737}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/f6ba195ee4592bf6f740ebc2060e927cc893b472/third_party/blink/renderer/core/layout/hit_test_result.h
[modify] https://crrev.com/f6ba195ee4592bf6f740ebc2060e927cc893b472/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object.cc
[modify] https://crrev.com/f6ba195ee4592bf6f740ebc2060e927cc893b472/third_party/blink/renderer/core/layout/svg/layout_svg_foreign_object_test.cc

Re #50, could you pls provide ETA for fix to be landed in trunk?
Ok, pls update the bug once cl lands in trunk also merge the change to canary branch #3449. Thank you.
Project Member

Comment 56 by bugdroid1@chromium.org, Jun 4 2018

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

commit 96f1ff917189c07e9a4002bf8a0cca0afe2b5031
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon Jun 04 20:11:48 2018

[ForeignObject] Apply an infinite cull rect when painting <foreignObject>.

Bug:  848179 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ibf31b9e4df4366363da473ee18d5ff319c1cc56b
Reviewed-on: https://chromium-review.googlesource.com/1085560
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564207}
[add] https://crrev.com/96f1ff917189c07e9a4002bf8a0cca0afe2b5031/third_party/WebKit/LayoutTests/svg/foreignObject/content-under-scroll-cull-rect-expected.html
[add] https://crrev.com/96f1ff917189c07e9a4002bf8a0cca0afe2b5031/third_party/WebKit/LayoutTests/svg/foreignObject/content-under-scroll-cull-rect.html
[modify] https://crrev.com/96f1ff917189c07e9a4002bf8a0cca0afe2b5031/third_party/blink/renderer/core/paint/svg_foreign_object_painter.cc

Project Member

Comment 57 by bugdroid1@chromium.org, Jun 4 2018

Labels: merge-merged-3449
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a609bccfdf4470dd2a54509e487cbb20c99aaea

commit 7a609bccfdf4470dd2a54509e487cbb20c99aaea
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon Jun 04 20:26:42 2018

[ForeignObject] Apply an infinite cull rect when painting <foreignObject>.

Bug:  848179 

TBR=chrishtr@chromium.org

(cherry picked from commit 96f1ff917189c07e9a4002bf8a0cca0afe2b5031)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ibf31b9e4df4366363da473ee18d5ff319c1cc56b
Reviewed-on: https://chromium-review.googlesource.com/1085560
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564207}
Reviewed-on: https://chromium-review.googlesource.com/1086031
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3449@{#3}
Cr-Branched-From: 5d909d4cf83d8082dc5adce7b55ebc89543bda94-refs/heads/master@{#563985}
[add] https://crrev.com/7a609bccfdf4470dd2a54509e487cbb20c99aaea/third_party/WebKit/LayoutTests/svg/foreignObject/content-under-scroll-cull-rect-expected.html
[add] https://crrev.com/7a609bccfdf4470dd2a54509e487cbb20c99aaea/third_party/WebKit/LayoutTests/svg/foreignObject/content-under-scroll-cull-rect.html
[modify] https://crrev.com/7a609bccfdf4470dd2a54509e487cbb20c99aaea/third_party/blink/renderer/core/paint/svg_foreign_object_painter.cc

Thank you Chris for the merge to canary branch 3449, triggered new canary (69.0.3449.2) from same branch.

Pls verify issue listed at #41 on canary version 69.0.3449.2 or higher.
The CL in #56 should be safe to merge to stable, because all it does is to
reduce culling, which should not affect correctness, just performance, and only
in a very minor way.
NextAction: 2018-06-05
Approving merge for CL listed at #56 to M67 branch 3396 based on comment #59 and per offline chat with chrishtr@. 

This will also need a merge to M68 branch 3440. Approving merge for M68 as well.

Pls do verify this bug on canary and update canary result tomorrow morning.
Cc: abdulsyed@chromium.org
+ abdulsyed@ (M68 Release TPM) as FYI
Approving merge for M68. Branch:3440
Project Member

Comment 63 by bugdroid1@chromium.org, Jun 4 2018

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

commit 12d037be7e41764f411734606ac47c90bbdd0302
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon Jun 04 21:56:58 2018

[ForeignObject] Apply an infinite cull rect when painting <foreignObject>.

Bug:  848179 

TBR=chrishtr@chromium.org

(cherry picked from commit 96f1ff917189c07e9a4002bf8a0cca0afe2b5031)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ibf31b9e4df4366363da473ee18d5ff319c1cc56b
Reviewed-on: https://chromium-review.googlesource.com/1085560
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564207}
Reviewed-on: https://chromium-review.googlesource.com/1086161
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#175}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[add] https://crrev.com/12d037be7e41764f411734606ac47c90bbdd0302/third_party/WebKit/LayoutTests/svg/foreignObject/content-under-scroll-cull-rect-expected.html
[add] https://crrev.com/12d037be7e41764f411734606ac47c90bbdd0302/third_party/WebKit/LayoutTests/svg/foreignObject/content-under-scroll-cull-rect.html
[modify] https://crrev.com/12d037be7e41764f411734606ac47c90bbdd0302/third_party/blink/renderer/core/paint/svg_foreign_object_painter.cc

Project Member

Comment 64 by bugdroid1@chromium.org, Jun 4 2018

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

commit d688af0c3741d01286c22cbfe6648a8494cdbe49
Author: Chris Harrelson <chrishtr@chromium.org>
Date: Mon Jun 04 21:59:59 2018

[ForeignObject] Apply an infinite cull rect when painting <foreignObject>.

Bug:  848179 

TBR=chrishtr@chromium.org

(cherry picked from commit 96f1ff917189c07e9a4002bf8a0cca0afe2b5031)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ibf31b9e4df4366363da473ee18d5ff319c1cc56b
Reviewed-on: https://chromium-review.googlesource.com/1085560
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#564207}
Reviewed-on: https://chromium-review.googlesource.com/1086207
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#741}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[add] https://crrev.com/d688af0c3741d01286c22cbfe6648a8494cdbe49/third_party/WebKit/LayoutTests/svg/foreignObject/content-under-scroll-cull-rect-expected.html
[add] https://crrev.com/d688af0c3741d01286c22cbfe6648a8494cdbe49/third_party/WebKit/LayoutTests/svg/foreignObject/content-under-scroll-cull-rect.html
[modify] https://crrev.com/d688af0c3741d01286c22cbfe6648a8494cdbe49/third_party/blink/renderer/core/paint/svg_foreign_object_painter.cc

Tested on 69.0.3449.2. Appears to be working there.

Comment 66 by ajha@chromium.org, Jun 5 2018

Labels: TE-Verified-69.0.3450.0 TE-Verified-M69
Thanks chrishtr@ for the fixes. Tested this on the latest canary 69.0.3450.0 on Windows-10, Mac OS 10.13.3 and Debian Rodete. 

All the scenarios i.e. C#0 jsbin(https://jsbin.com/qozapirone/edit?html,output), C#0 html file and duped  Issue 848539  https://jsfiddle.net/ss9j4ybv/3/  seems to be working fine. Attaching the screen-cast of the same and adding the verified label for M-69.
848179_Canary_Verification.webm
5.8 MB View Download
The NextAction date has arrived: 2018-06-05
Status: Fixed (was: Assigned)
Marking as fixed.

Comment 69 by ajha@chromium.org, Jun 6 2018

Labels: TE-Verified-M67 TE-Verified-67.0.3396.79
Verified the fix on the latest M-67 i.e. 67.0.3396.79 on Windows-10, Mac OS 10.13.3 and Debian Rodete and all the scenarios seems to be working fine there. Hence adding the verified label for M-67.
Good news: 67.0.3396.79 is now available with the fix on Chrome Stable. You can force an update by visiting chrome://chrome. Thank you.
Requesting postmortem for this bug. Please see go/chrome-postmortems for the process to follow.
Re #71: Acknowledged, will write one.
 Issue 849992  has been merged into this issue.

Comment 74 by ajha@chromium.org, Jun 7 2018

Labels: TE-Verified-M68 TE-Verified-68.0.3440.17
Verified the merge in the latest M-68 i.e 68.0.3440.17 on Windows-10, Mac OS 10.13.3 and Debian Rodete as per C#41 and all the scenario seems to be working fine. Hence adding the verified label for M-68 as well.
The problem is solved for our customers, thanks everyone!
 Issue 850540  has been merged into this issue.
 Issue 866297  has been merged into this issue.

Comment 78 Deleted

Was a postmortem ever completed for this bug? (requested in comment #71)

After this bug was fixed, a similar issue came up again in https://bugs.chromium.org/p/chromium/issues/detail?id=908570
...which is now affecting customers of Chrome 71.

Sign in to add a comment