1979474 - Crash in [@ mozilla::dom::RangeBasedTextDirectiveCreator::CollectContextTerms]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/227f8e9f-a8ef-4f4a-8c6c-b1cbb0250725

MOZ_CRASH Reason:

MOZ_DIAGNOSTIC_ASSERT(lastBlockBoundaryInRange.isSome()) (If the target range contains a block boundary looking left-to-right, it must also contain one looking right-to-left)

Top 10 frames:

0  xul.dll  mozilla::dom::RangeBasedTextDirectiveCreator::CollectContextTerms()  dom/base/TextDirectiveCreator.cpp:209
1  xul.dll  mozilla::dom::TextDirectiveCreator::CreateTextDirectiveFromRange(mozilla::dom...  dom/base/TextDirectiveCreator.cpp:51
2  xul.dll  mozilla::dom::FragmentDirective::CreateTextDirectiveForSelection()  dom/base/FragmentDirective.cpp:468
3  xul.dll  mozilla::dom::FragmentDirective_Binding::createTextDirectiveForSelection(JSCo...  dom/bindings/FragmentDirectiveBinding.cpp:145
3  xul.dll  mozilla::dom::FragmentDirective_Binding::createTextDirectiveForSelection_prom...  dom/bindings/FragmentDirectiveBinding.cpp:156
4  xul.dll  mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::Nor...  dom/bindings/BindingUtils.cpp:3309
5  xul.dll  CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::...  js/src/vm/Interpreter.cpp:501
5  xul.dll  js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstru...  js/src/vm/Interpreter.cpp:597
5  xul.dll  InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason)  js/src/vm/Interpreter.cpp:664
5  xul.dll  js::CallFromStack(JSContext*, JS::CallArgs const&, js::CallReason)  js/src/vm/Interpreter.cpp:669

Bug 1971670 fixed a crash with this signature but it is still happening.

Comment 1

[blinky]

Steps to reproduce:

1. Go to https://phabricator.services.mozilla.com/D53689.
2. Select the patch on the left side.
3. Right-click on the selection.

Actual results:

The tab crashed.

Crash report: bp-75d098b6-2195-4fd9-900c-4c7f50250728

Regression range:
https://hg-edge.mozilla.org/integration/autoland/pushloghtml?fromchange=7336d25e87e73c24c9c5a83e44d8121870b97c53&tochange=79170f140f14b8cc4b668e6137a623118ddc49cf

Regressed by: bug 1976480

Comment 2

[Andrew McCreight [:mccr8]]

Thank you for the STR and regression range! I guess 142 isn't affected yet.

Comment 3

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1976480

Comment 4

[Jan Jaeschke [:jjaschke]]

That's awesome, thank you for giving the STR! This would have been really hard to find just from the crash report.

It seems that the issue here is an empty <td>. I can reproduce with this trivial example:

<table>
  <tr>
    <td>}</td>
    <td></td>
  </tr>
</table>

Try it out:

data:text/html,<table><tr><td>}</td><td></td></tr></table>

This crashes only if the selection is created by dragging. Triple-click or CTRL-A works fine.

In case of the drag selection, the start point of the range is the text inside the first <td> at offset 0, and the end is the second <td> with offset 0.
Because of this, it is wrongfully decided here to use range-based matching.
The reason why this occurs is because bug 1976480 returns early when "normalizing" the range to proper boundary points in case the range content is only punctuation. This skips these calls, which move the boundary points to the nearest text nodes.

Comment 5

[Jan Jaeschke [:jjaschke]]

Attachment: Bug 1979474 - Text Fragments: Normalize range boundaries when extending to word boundaries in the punctuation-only case. r=#dom-core!

Bug 1976480 introduced a special code path if the target range only consists of punctuation.
However, this new code path did not "normalize" the boundary points, ie. moving them to
the actual text node. Therefore, the target range could contain empty blocks
(such as an empty <td>), which would then trigger range-based matching.

This patch adds normalizing to the target range in case of punctuation-only ranges.

Comment 6

[BugBot [:suhaib / :marco]]

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 10 desktop browser crashes on nightly

:jjaschke, could you consider increasing the severity of this top-crash bug?

For more information, please visit BugBot documentation.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8bc85faf7d7f
https://hg.mozilla.org/integration/autoland/rev/87e0ba646802
Text Fragments: Normalize range boundaries when extending to word boundaries in the punctuation-only case. r=dom-core,mccr8

Comment 8

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/87e0ba646802