Issue metadata
Sign in to add a comment
|
foreignObject hit test offset is wrong
Reported by
batist.l...@gmail.com,
May 31 2018
|
|||||||||||||||||||||||||
Issue descriptionUserAgent: 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)
,
May 31 2018
,
May 31 2018
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.
,
May 31 2018
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.
,
May 31 2018
I have users reporting this problem using Windows 7.
,
May 31 2018
Thanks for the Windows 7 report.
,
May 31 2018
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?
,
May 31 2018
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.
,
May 31 2018
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.
,
May 31 2018
,
May 31 2018
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@.
,
May 31 2018
,
May 31 2018
,
Jun 1 2018
,
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
,
Jun 1 2018
,
Jun 1 2018
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
,
Jun 1 2018
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
,
Jun 1 2018
Thank you chrishtr@ for merging to canary branch 3447. I'm triggering new canary from same branch.
,
Jun 1 2018
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.
,
Jun 1 2018
,
Jun 1 2018
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!
,
Jun 1 2018
The remaining bug is that PointInInnerNodeFrame is not being propagated back properly into the ancestor HitTestResult struct.
,
Jun 1 2018
Almost done with bugfix CL.
,
Jun 1 2018
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?
,
Jun 1 2018
Yes, the plan is to merge fixes to all the issues to 67. (and 68)
,
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
,
Jun 1 2018
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.
,
Jun 2 2018
Can anyone try to repo this issue on Android?
,
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?
,
Jun 2 2018
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
,
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
,
Jun 2 2018
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
,
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
,
Jun 2 2018
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.
,
Jun 2 2018
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.
,
Jun 3 2018
Just tried on 69.0.3447.5, looks fixed to me.
,
Jun 3 2018
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.
,
Jun 4 2018
The NextAction date has arrived: 2018-06-04
,
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!
,
Jun 4 2018
Thank you ajha@. chrishtr@, ptal comment #41 before merging to M67. Thank you.
,
Jun 4 2018
Looking into it.
,
Jun 4 2018
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
,
Jun 4 2018
,
Jun 4 2018
The issue reported in #41 is yet another issue, unrelated to the other ones. :( I have a fix for that as well.
,
Jun 4 2018
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.
,
Jun 4 2018
Yes, ideally that fix would also go into M67. I agree that this is a bad situation. :(
,
Jun 4 2018
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.
,
Jun 4 2018
Ok. Note that the last fix that is not yet committed is really safe and will not introduce any new bugs.
,
Jun 4 2018
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
,
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
,
Jun 4 2018
Re #50, could you pls provide ETA for fix to be landed in trunk?
,
Jun 4 2018
,
Jun 4 2018
Ok, pls update the bug once cl lands in trunk also merge the change to canary branch #3449. Thank you.
,
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
,
Jun 4 2018
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
,
Jun 4 2018
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.
,
Jun 4 2018
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.
,
Jun 4 2018
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.
,
Jun 4 2018
+ abdulsyed@ (M68 Release TPM) as FYI
,
Jun 4 2018
Approving merge for M68. Branch:3440
,
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
,
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
,
Jun 5 2018
Tested on 69.0.3449.2. Appears to be working there.
,
Jun 5 2018
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.
,
Jun 5 2018
The NextAction date has arrived: 2018-06-05
,
Jun 5 2018
Marking as fixed.
,
Jun 6 2018
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.
,
Jun 6 2018
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.
,
Jun 7 2018
Requesting postmortem for this bug. Please see go/chrome-postmortems for the process to follow.
,
Jun 7 2018
Re #71: Acknowledged, will write one.
,
Jun 7 2018
Issue 849992 has been merged into this issue.
,
Jun 7 2018
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.
,
Jun 7 2018
The problem is solved for our customers, thanks everyone!
,
Jun 7 2018
Issue 850540 has been merged into this issue.
,
Jul 23
Issue 866297 has been merged into this issue.
,
Dec 12
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 |
||||||||||||||||||||||||||
Comment 1 by gov...@chromium.org
, May 31 2018Labels: Needs-Triage-M67