Title: [274203] trunk/Source/WebCore
- Revision
- 274203
- Author
- [email protected]
- Date
- 2021-03-10 00:19:08 -0800 (Wed, 10 Mar 2021)
Log Message
Release assertion failures under Editor::scanSelectionForTelephoneNumbers
https://bugs.webkit.org/show_bug.cgi?id=223016
<rdar://problem/73159921>
Reviewed by Ryosuke Niwa.
No new tests; speculative fix for a non-reproducible crash, which theoretically
has been avoided in a second way on trunk.
* editing/Editor.cpp:
(WebCore::extendSelection):
(WebCore::Editor::scanSelectionForTelephoneNumbers):
In r272777, Ryosuke discovered a case where FrameSelection::isRange()
can be true, but firstRange() is an invalid range; in testing, forcing
this to be the case reproduces the crash as reported.
While that change may have fixed the root cause of this crash, we don't
know that we've found all of the ways that one can get a orphaned
selection *into* FrameSelection, and want a less risky workaround for
this crash, so we'll also fix it in scanSelectionForTelephoneNumbers,
by null-checking the result of FrameSelection::firstRange().
Also null-check the result of extendSelection(), since I cannot prove for sure
that a valid range cannot become invalid in this method.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (274202 => 274203)
--- trunk/Source/WebCore/ChangeLog 2021-03-10 07:43:55 UTC (rev 274202)
+++ trunk/Source/WebCore/ChangeLog 2021-03-10 08:19:08 UTC (rev 274203)
@@ -1,3 +1,30 @@
+2021-03-10 Tim Horton <[email protected]>
+
+ Release assertion failures under Editor::scanSelectionForTelephoneNumbers
+ https://bugs.webkit.org/show_bug.cgi?id=223016
+ <rdar://problem/73159921>
+
+ Reviewed by Ryosuke Niwa.
+
+ No new tests; speculative fix for a non-reproducible crash, which theoretically
+ has been avoided in a second way on trunk.
+
+ * editing/Editor.cpp:
+ (WebCore::extendSelection):
+ (WebCore::Editor::scanSelectionForTelephoneNumbers):
+ In r272777, Ryosuke discovered a case where FrameSelection::isRange()
+ can be true, but firstRange() is an invalid range; in testing, forcing
+ this to be the case reproduces the crash as reported.
+
+ While that change may have fixed the root cause of this crash, we don't
+ know that we've found all of the ways that one can get a orphaned
+ selection *into* FrameSelection, and want a less risky workaround for
+ this crash, so we'll also fix it in scanSelectionForTelephoneNumbers,
+ by null-checking the result of FrameSelection::firstRange().
+
+ Also null-check the result of extendSelection(), since I cannot prove for sure
+ that a valid range cannot become invalid in this method.
+
2021-03-09 Venky Dass <[email protected]>
Nullptr crash in Node::isTextNode() via ApplyBlockElementCommand::endOfNextParagraphSplittingTextNodesIfNeeded
Modified: trunk/Source/WebCore/editing/Editor.cpp (274202 => 274203)
--- trunk/Source/WebCore/editing/Editor.cpp 2021-03-10 07:43:55 UTC (rev 274202)
+++ trunk/Source/WebCore/editing/Editor.cpp 2021-03-10 08:19:08 UTC (rev 274203)
@@ -3677,7 +3677,7 @@
return result;
}
-static SimpleRange extendSelection(const SimpleRange& range, unsigned charactersToExtend)
+static Optional<SimpleRange> extendSelection(const SimpleRange& range, unsigned charactersToExtend)
{
auto start = makeDeprecatedLegacyPosition(range.start);
auto end = makeDeprecatedLegacyPosition(range.end);
@@ -3685,7 +3685,7 @@
start = start.previous(Character);
end = end.next(Character);
}
- return *makeSimpleRange(start, end);
+ return makeSimpleRange(start, end);
}
void Editor::scanSelectionForTelephoneNumbers()
@@ -3698,16 +3698,19 @@
auto& selection = m_document.selection();
if (selection.isRange()) {
- auto selectedRange = *selection.selection().firstRange();
- // Extend the range a few characters in each direction to detect incompletely selected phone numbers.
- constexpr unsigned charactersToExtend = 15;
- for (auto& range : scanForTelephoneNumbers(extendSelection(selectedRange, charactersToExtend))) {
- // FIXME: Why do we do this unconditionally instead of when only when it overlaps the selection?
- addMarker(range, DocumentMarker::TelephoneNumber);
+ if (auto selectedRange = selection.selection().firstRange()) {
+ // Extend the range a few characters in each direction to detect incompletely selected phone numbers.
+ constexpr unsigned charactersToExtend = 15;
+ if (auto extendedRange = extendSelection(*selectedRange, charactersToExtend)) {
+ for (auto& range : scanForTelephoneNumbers(*extendedRange)) {
+ // FIXME: Why do we do this unconditionally instead of when only when it overlaps the selection?
+ addMarker(range, DocumentMarker::TelephoneNumber);
- // Only consider ranges with a detected telephone number if they overlap with the selection.
- if (intersects<ComposedTree>(range, selectedRange))
- m_detectedTelephoneNumberRanges.append(range);
+ // Only consider ranges with a detected telephone number if they overlap with the selection.
+ if (intersects<ComposedTree>(range, *selectedRange))
+ m_detectedTelephoneNumberRanges.append(range);
+ }
+ }
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
