[LayoutNG] Fix hit testing on first letter [chromium/src : master]
Xiaocheng Hu (Gerrit)
Xiaocheng Hu uploaded patch set #6 to this change.
[LayoutNG] Fix hit testing on first letter
If an NGPhysicalTextFragment is created from first-letter, its
GetNode() method returns nullptr, which leads to false negative
when hit-testing on this fragment.
This patch adds a new method to obtain the text node for first-
letter fragments, and applies it in hit test code, so that we
can correctly obtain the text node when hit-testing the first
letter.
Bug: 811502
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I70e5cd4948f248a6e51e3a1732273779ef36509c
---
M third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
M third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.cc
M third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h
M third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc
4 files changed, 15 insertions(+), 3 deletions(-)
To view, visit change 1072691. To unsubscribe, or for help writing mail filters, visit settings.
Xiaocheng Hu (Gerrit)
Xiaocheng Hu would like Yoichi Osato, Emil A Eklund, Yoshifumi Inoue and Koji Ishii to review this change.
Xiaocheng Hu (Gerrit)
PTAL.
Please also let me know if you can find a better name for the new method. Thanks :)
Patch set 6:Commit-Queue +1
Yoshifumi Inoue (Gerrit)
lgtm
Reasonable.
Patch set 6:Code-Review +1
Yoichi Osato (Gerrit)
Emil A Eklund (Gerrit)
Patch set 6:Code-Review +1
1 comment:
File third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h:
Patch Set #6, Line 142:
Node* GetTextSourceNode() const;(optional) Can we drop the Get prefix and have this be "TextSourceNode()" instead?
To view, visit change 1072691. To unsubscribe, or for help writing mail filters, visit settings.
Koji Ishii (Gerrit)
lgtm w/nit
File third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.cc:
Patch Set #6, Line 186:
return GetNode();Minor, but "TextSourceNode()" returning non-text node isn't intuitive. Should this return "Text*" instead and return nullptr if it's not a text node, and caller checks nullptr?
To view, visit change 1072691. To unsubscribe, or for help writing mail filters, visit settings.
Xiaocheng Hu (Gerrit)
Xiaocheng Hu uploaded patch set #7 to this change.
[LayoutNG] Fix hit testing on first letter
If an NGPhysicalTextFragment is created from first-letter, its
GetNode() method returns nullptr, which leads to false negative
when hit-testing on this fragment.
This patch adds a new method to obtain the text node for first-
letter fragments, and applies it in hit test code, so that we
can correctly obtain the text node when hit-testing the first
letter.
Bug: 811502
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I70e5cd4948f248a6e51e3a1732273779ef36509c
---
M third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
M third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.cc
M third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h
M third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc
4 files changed, 15 insertions(+), 3 deletions(-)
To view, visit change 1072691. To unsubscribe, or for help writing mail filters, visit settings.
Xiaocheng Hu (Gerrit)
Thanks for reviewing!
Patch set 7:Commit-Queue +2
2 comments:
File third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h:
Patch Set #6, Line 142:
Node* TextSourceNode() const;
(optional) Can we drop the Get prefix and have this be "TextSourceNode()" instead?
Sounds better. Done.
File third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.cc:
Patch Set #6, Line 186:
return GetNode();
Minor, but "TextSourceNode()" returning non-text node isn't intuitive. […]
We might get non-text node from this method, e.g. <br>, so the return type can't be |Text*|
To view, visit change 1072691. To unsubscribe, or for help writing mail filters, visit settings.
Commit Bot (Gerrit)
CQ is trying the patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"" https://chromium-review.googlesource.com/c/1072691/7
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/1072691/7
Bot data: {"action": "start", "triggered_at": "2018-05-25T18:33:02.0Z", "cq_cfg_revision": "6a25833cb93164fb378d8087f9dae5fc28fc9b1e", "revision": "24e164939ea78445e3b269a280799530c1d2113a"}
Commit Bot (Gerrit)
Try jobs failed on following builders:
linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/559868)
Xiaocheng Hu (Gerrit)
Commit Bot (Gerrit)
CQ is trying the patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"" https://chromium-review.googlesource.com/c/1072691/7
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/1072691/7
Bot data: {"action": "start", "triggered_at": "2018-05-25T20:58:06.0Z", "cq_cfg_revision": "6a25833cb93164fb378d8087f9dae5fc28fc9b1e", "revision": "24e164939ea78445e3b269a280799530c1d2113a"}
To view, visit change 1072691. To unsubscribe, or for help writing mail filters, visit settings.
Xiaocheng Hu (Gerrit)
Xiaocheng Hu removed a vote from this change.
To view, visit change 1072691. To unsubscribe, or for help writing mail filters, visit settings.
Xiaocheng Hu (Gerrit)
Commit Bot (Gerrit)
CQ is trying the patch.
Note: The patchset sent to CQ was uploaded after this CL was approved.
"" https://chromium-review.googlesource.com/c/1072691/7
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/1072691/7
Bot data: {"action": "start", "triggered_at": "2018-05-25T21:41:19.0Z", "cq_cfg_revision": "6a25833cb93164fb378d8087f9dae5fc28fc9b1e", "revision": "24e164939ea78445e3b269a280799530c1d2113a"}
To view, visit change 1072691. To unsubscribe, or for help writing mail filters, visit settings.
Commit Bot (Gerrit)
Commit Bot merged this change.
[LayoutNG] Fix hit testing on first letter
If an NGPhysicalTextFragment is created from first-letter, its
GetNode() method returns nullptr, which leads to false negative
when hit-testing on this fragment.
This patch adds a new method to obtain the text node for first-
letter fragments, and applies it in hit test code, so that we
can correctly obtain the text node when hit-testing the first
letter.
Bug: 811502
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I70e5cd4948f248a6e51e3a1732273779ef36509c
Reviewed-on: https://chromium-review.googlesource.com/1072691
Commit-Queue: Xiaocheng Hu <xiaoc...@chromium.org>
Reviewed-by: Yoshifumi Inoue <yo...@chromium.org>
Reviewed-by: Yoichi Osato <yoi...@chromium.org>
Reviewed-by: Emil A Eklund <e...@chromium.org>
Reviewed-by: Koji Ishii <ko...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562030}
---
M third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
M third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.cc
M third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h
M third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc
4 files changed, 15 insertions(+), 3 deletions(-)