Render crashes (ReceivedBadMessage) in RenderFrameHostImpl::OnContextMenu for editable div
Reported by
vladbe...@yandex-team.ru,
May 28 2018
|
||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3444.0 Safari/537.36 Example URL: On forums with editable divs, the simplified version is attached Steps to reproduce the problem: 1. Open the attachment page - crash_cm.html (it's the simplified version of the forum answer form). 2. Select from point 1. to point 2., see the crash_select.png attachment (to have a blue rectangle on the left side). 3. Click RMB on the selected text. What is the expected behavior? Context menu is opening correctly. What went wrong? Renderer crashed (ReceivedBadMessage) on this line: https://chromium.googlesource.com/chromium/src/+/c30a610a3f409fe58aa05c9092096ad08d229904/content/browser/frame_host/render_frame_host_impl.cc#2037 Does it occur on multiple sites: Yes Is it a problem with a plugin? No Did this work before? Yes 60.0.3110.0 Does this work in other browsers? Yes Chrome version: 69.0.3444.0 Channel: canary OS Version: 10.0 Flash Version: It also crashes for 66.0.3338.0, but works for 60.0.3110.0.
,
May 28 2018
,
May 29 2018
,
May 29 2018
Able to reproduce the issue on Windows 10, mac 10.13.3 and Ubuntu 17.10 using chrome reported version #69.0.3444.0. Bisect Information: ===================== Good build: 64.0.3261.0 Bad Build : 64.0.3262.0 Change Log URL: https://chromium.googlesource.com/chromium/src/+log/978f3c4fb8e40a697b9d83736f127287114f9af2..4d95029f8eeb98144483e383293a27f91534e73d From the above change log suspecting below change Change-Id: I18005806b0e8368c9fe72aec4f40ccc249016e0e Reviewed-on: https://chromium-review.googlesource.com/720270 ctzsm@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Thanks...!!
,
May 29 2018
I can also repro this on Android with Bluetooth mouse. We rejected/killed the render because we believe the parameter |selection_start_offset| passed to browser is not valid. Looks like there is something wrong with |frame->GetInputMethodController().GetSelectionOffsets()| with this special case. From my investigation, both range.IsNull() and range.IsEmpty() are true here, which shouldn't since I can get the correct selection text. I tried to remove my check locally, however, it will eventually fail on other DCHECKs on Android. Example 1: [FATAL:web_surrounding_text.cc(78)] Check failed: RootEditableElementOf(start_position) == RootEditableElementOf(end_position) (DIV class="ipsQuote_contents ipsClearfix cke_widget_editable" (editable) (focused) vs. DIV class="cke_reset cke_enable_context_menu cke_editable cke_editable_themed cke_contents_ltr" (editable)) Example 2: [FATAL:ephemeral_range.cc(42)] Check failed: start_position_.IsValidFor(*start_position_.GetDocument()). I didn't see the crash on Linux if I removed my check. The HTML attached is fairly complex, it has nested contenteditable="true" then contenteditable="false" then contenteditable="true" and a sibling element to the contenteditable="false" element, we probably didn't handle this case very well even before my change.
,
May 31 2018
,
Jun 1 2018
Thank you for your attention! I made a simpler version without styles (attached). As ctzsm@ mentioned, the renderer indeed crashes when a document has following structure: div contenteditable="true" > div contenteditable="false" > div contenteditable="true". It happens relatively often.
,
Jun 2 2018
The current behavior is actually not correct, we shouldn't allow cross boundary selection at all. To be more specific, when you start the selection from the the very outer <div contenteditable="true" />, and trying to expand the selection range to <div contenteditable="false" /> element, it should atomically select the whole <div contenteditable="false" /> element. The current behavior allows you to penetrate into this <div contenteditable="false">, which is wrong. I'll have a fix to prevent this behavior.
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f77757625aff3f89b2c2b5d01294e5407a284cd commit 6f77757625aff3f89b2c2b5d01294e5407a284cd Author: Shimi Zhang <ctzsm@chromium.org> Date: Tue Jun 19 20:54:00 2018 Convert editing/deleting/5115601.html to utilize |selection_test()| In preparing for http://crrev/c/1102157, make it easier for us to handle the behavior change. Bug: 847192 Change-Id: I5b4053056e96b7c2ecec879b3fea722b3d68d511 Reviewed-on: https://chromium-review.googlesource.com/1105575 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Shimi Zhang <ctzsm@chromium.org> Cr-Commit-Position: refs/heads/master@{#568598} [delete] https://crrev.com/617fed5334a155cac8da954df71481afbbab4a89/third_party/WebKit/LayoutTests/editing/deleting/5115601-expected.txt [modify] https://crrev.com/6f77757625aff3f89b2c2b5d01294e5407a284cd/third_party/WebKit/LayoutTests/editing/deleting/5115601.html
,
Jun 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d800ce8818f2a9dcd873c92ca30809e4f1c1132f commit d800ce8818f2a9dcd873c92ca30809e4f1c1132f Author: Shimi Zhang <ctzsm@chromium.org> Date: Tue Jun 19 20:55:57 2018 Convert delete-across-editable-content-boundaries-2 test This CL converts delete-across-editable-content-boundaries-2 to utilize |selection_test()|. In preparing for http://crrev/c/1102157, make it easier for us to handle the behavior change. Bug: 847192 Change-Id: I85ace72d163656fed187b679f287fed2c053d227 Reviewed-on: https://chromium-review.googlesource.com/1106547 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Shimi Zhang <ctzsm@chromium.org> Cr-Commit-Position: refs/heads/master@{#568599} [delete] https://crrev.com/6f77757625aff3f89b2c2b5d01294e5407a284cd/third_party/WebKit/LayoutTests/editing/deleting/delete-across-editable-content-boundaries-2-expected.txt [modify] https://crrev.com/d800ce8818f2a9dcd873c92ca30809e4f1c1132f/third_party/WebKit/LayoutTests/editing/deleting/delete-across-editable-content-boundaries-2.html
,
Jun 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/88f5558daf34d4ffb7abaea7818f0b29d2dc48e8 commit 88f5558daf34d4ffb7abaea7818f0b29d2dc48e8 Author: Shimi Zhang <ctzsm@chromium.org> Date: Wed Jun 20 01:47:22 2018 Convert editing/deleting/6026335.html to utilize |selection_test()| In preparing for http://crrev/c/1102157, make it easier for us to handle the behavior change. Bug: 847192 Change-Id: I0f67b894e38737339efa2aafe3093324876f5368 Reviewed-on: https://chromium-review.googlesource.com/1106598 Commit-Queue: Shimi Zhang <ctzsm@chromium.org> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#568690} [delete] https://crrev.com/20579735a6a48710514d69d50ed0d2516b97f3ae/third_party/WebKit/LayoutTests/editing/deleting/6026335-expected.txt [modify] https://crrev.com/88f5558daf34d4ffb7abaea7818f0b29d2dc48e8/third_party/WebKit/LayoutTests/editing/deleting/6026335.html
,
Jun 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2b87b19cca0516bd754c7bc17462ff4bee9c4a9b commit 2b87b19cca0516bd754c7bc17462ff4bee9c4a9b Author: Shimi Zhang <ctzsm@chromium.org> Date: Wed Jun 20 21:43:38 2018 Fix "^" and "|" misuse Wrongly used "^" and "|" in two tests, so the selection direction is not matching the old tests. This CL fixed these misuse. Bug: 847192 Change-Id: I036c3a00eacb9ff4a7d8244ec40b3899aa68feb3 Reviewed-on: https://chromium-review.googlesource.com/1108742 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Shimi Zhang <ctzsm@chromium.org> Cr-Commit-Position: refs/heads/master@{#569034} [modify] https://crrev.com/2b87b19cca0516bd754c7bc17462ff4bee9c4a9b/third_party/WebKit/LayoutTests/editing/deleting/6026335.html [modify] https://crrev.com/2b87b19cca0516bd754c7bc17462ff4bee9c4a9b/third_party/WebKit/LayoutTests/editing/deleting/delete-across-editable-content-boundaries-2.html
,
Jul 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7bd29404a6ab8d36bdff4123ae522fcd9068344b commit 7bd29404a6ab8d36bdff4123ae522fcd9068344b Author: Shimi Zhang <ctzsm@chromium.org> Date: Wed Jul 18 08:09:47 2018 [Blink] Avoid crossing editing boundaries selection. crbug.com/847192 exposed a case that the original |AdjustSelectionToAvoidCrossingEditingBoundaries()| doesn't work correctly sometimes. We worked out a general algorithm to replace it for selections in DOM tree. See doc https://goo.gl/d7eNB2 for the final version. There are still issues for this adjustment in flat tree selection due to shadow DOM related cases. See discussion http://bit.ly/2Nb4n9l. Tests: Tests converted to unit tests due to MostBackwardCaretPosition() 1) editing/execCommand/delete-non-editable-range-crash.html, 2) editing/execCommand/format-block-contenteditable-false.html, Tests were expecting wrong behavior 3) editing/deleting/6026335-expected.txt, 4) editing/deleting/5115601.html, 5) editing/deleting/5144139-2.html, 6) editing/style/apply_style_to_plaintext_only.html, 7) editing/deleting/delete-across-editable-content-boundaries-3.html, 8) editing/deleting/delete-across-editable-content-boundaries-2-expected.txt Re-enabled old disabled tests 9) SelectRangeCanMoveSelection{Start,End}. 10) Added more unit tests to selection_adjuster_test.cc Bug: 847192 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I4f52898cfa0dbbcc4cbc2b6b107f8eb81de9ac84 Reviewed-on: https://chromium-review.googlesource.com/1102157 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Reviewed-by: Yoichi Osato <yoichio@chromium.org> Commit-Queue: Shimi Zhang <ctzsm@chromium.org> Cr-Commit-Position: refs/heads/master@{#575975} [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/deleting/5115601.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/deleting/5144139-2.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/deleting/6026335.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/deleting/delete-across-editable-content-boundaries-2.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/deleting/delete-across-editable-content-boundaries-3.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/execCommand/delete-non-editable-range-crash.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/execCommand/format-block-contenteditable-false.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/style/apply_style_to_plaintext_only.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/WebKit/LayoutTests/editing/style/justify-left-crash.html [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/blink/renderer/core/editing/selection_adjuster.cc [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/blink/renderer/core/editing/selection_adjuster_test.cc [modify] https://crrev.com/7bd29404a6ab8d36bdff4123ae522fcd9068344b/third_party/blink/renderer/core/exported/web_frame_test.cc
,
Jul 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b991e98fe3af06c601681332680a19c77d0aec25 commit b991e98fe3af06c601681332680a19c77d0aec25 Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com> Date: Thu Jul 19 17:30:12 2018 editing: Fix build with GCC after #575975 GCC currently fails to build selection_adjuster.cc after 7bd29404a ("[Blink] Avoid crossing editing boundaries selection"): ../../third_party/blink/renderer/core/editing/selection_adjuster.cc:607:13: error: explicit specialization in non-namespace scope ‘class blink::EditingBoundaryAdjuster’ template <> ^ ../../third_party/blink/renderer/core/editing/selection_adjuster.cc:611:37: error: template-id ‘IsEditingBoundary<blink::EditingInFlatTreeStrategy>’ in declaration of primary template bool is_previous_node_editable) { ^ which can be fixed by specializing EditingBoundaryAdjuster::IsEditingBoundary() outside of EditingBoundaryAdjuster's declaration. Bug: 819294, 847192 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng Change-Id: I624d693c4b629aefc5ada45313cdcd7242a00a2d Reviewed-on: https://chromium-review.googlesource.com/1143393 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Raphael Kubo da Costa (CET) <raphael.kubo.da.costa@intel.com> Cr-Commit-Position: refs/heads/master@{#576542} [modify] https://crrev.com/b991e98fe3af06c601681332680a19c77d0aec25/third_party/blink/renderer/core/editing/selection_adjuster.cc
,
Jul 20
Should be fixed on M69, mark as fixed.
,
Jul 23
Able to reproduce the issue on Mac 10.13.3 using chrome reported version #69.0.3444.0. Unable to verify the issue on mac 10.13.3, win-10 and ubuntu 17.10 as per the comment #0 using crash_cm.html as unable to select from point 1. to point 2., as in the crash_select.png attachment. Attaching screen cast for reference. ctzsm@ - Could you please provide any other sample file to verify the issue from TE-end. Thanks...!!
,
Jul 30
krajshree@, unable to select from point 1 to point 2 is the expected behavior. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by vladbe...@yandex-team.ru
, May 28 2018