- Revision
- 291201
- Author
- wenson_hs...@apple.com
- Date
- 2022-03-11 16:56:15 -0800 (Fri, 11 Mar 2022)
Log Message
Refactor the filter block in DocumentMarkerController::filterMarkers() to return an enum type
https://bugs.webkit.org/show_bug.cgi?id=237794
Reviewed by Megan Gardner.
Source/WebCore:
Change the return value of the `filter` function argument in `filterMarkers()` from a `bool` to a named enum
type with 2 values: `Keep` or `Remove`; this makes it more obvious to call sites whether the return value will
cause document markers to remain or be removed from the document.
* dom/DocumentMarkerController.cpp:
(WebCore::DocumentMarkerController::filterMarkers):
(WebCore::DocumentMarkerController::removeMarkers):
* dom/DocumentMarkerController.h: Remove a FIXME and comment that's no longer needed.
Source/WebKit:
Drive-by fix: only remove the spellchecking document marker in `removeAnnotationRelativeToSelection` if the
annotation string of the marker matches the given annotation string to the method.
* WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:
(WebKit::TextCheckingControllerProxy::removeAnnotationRelativeToSelection):
* WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
(WebKit::WebPage::clearDictationAlternatives):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (291200 => 291201)
--- trunk/Source/WebCore/ChangeLog 2022-03-12 00:32:18 UTC (rev 291200)
+++ trunk/Source/WebCore/ChangeLog 2022-03-12 00:56:15 UTC (rev 291201)
@@ -1,3 +1,19 @@
+2022-03-11 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Refactor the filter block in DocumentMarkerController::filterMarkers() to return an enum type
+ https://bugs.webkit.org/show_bug.cgi?id=237794
+
+ Reviewed by Megan Gardner.
+
+ Change the return value of the `filter` function argument in `filterMarkers()` from a `bool` to a named enum
+ type with 2 values: `Keep` or `Remove`; this makes it more obvious to call sites whether the return value will
+ cause document markers to remain or be removed from the document.
+
+ * dom/DocumentMarkerController.cpp:
+ (WebCore::DocumentMarkerController::filterMarkers):
+ (WebCore::DocumentMarkerController::removeMarkers):
+ * dom/DocumentMarkerController.h: Remove a FIXME and comment that's no longer needed.
+
2022-03-11 Alex Christensen <achristen...@webkit.org>
Finish implementing modify-headers actions for WKContentRuleList SPI
Modified: trunk/Source/WebCore/dom/DocumentMarkerController.cpp (291200 => 291201)
--- trunk/Source/WebCore/dom/DocumentMarkerController.cpp 2022-03-12 00:32:18 UTC (rev 291200)
+++ trunk/Source/WebCore/dom/DocumentMarkerController.cpp 2022-03-12 00:56:15 UTC (rev 291201)
@@ -92,7 +92,7 @@
filterMarkers(range, nullptr, types, overlapRule);
}
-void DocumentMarkerController::filterMarkers(const SimpleRange& range, const Function<bool(const DocumentMarker&)>& filter, OptionSet<DocumentMarker::MarkerType> types, RemovePartiallyOverlappingMarker overlapRule)
+void DocumentMarkerController::filterMarkers(const SimpleRange& range, const Function<FilterMarkerResult(const DocumentMarker&)>& filter, OptionSet<DocumentMarker::MarkerType> types, RemovePartiallyOverlappingMarker overlapRule)
{
for (auto& textPiece : collectTextRanges(range)) {
if (!possiblyHasMarkers(types))
@@ -357,7 +357,7 @@
}
}
-void DocumentMarkerController::removeMarkers(Node& node, OffsetRange range, OptionSet<DocumentMarker::MarkerType> types, const Function<bool(const DocumentMarker&)>& filter, RemovePartiallyOverlappingMarker overlapRule)
+void DocumentMarkerController::removeMarkers(Node& node, OffsetRange range, OptionSet<DocumentMarker::MarkerType> types, const Function<FilterMarkerResult(const DocumentMarker&)>& filter, RemovePartiallyOverlappingMarker overlapRule)
{
if (range.start >= range.end)
return;
@@ -384,7 +384,7 @@
continue;
}
- if (filter && !filter(marker)) {
+ if (filter && filter(marker) == FilterMarkerResult::Keep) {
i++;
continue;
}
Modified: trunk/Source/WebCore/dom/DocumentMarkerController.h (291200 => 291201)
--- trunk/Source/WebCore/dom/DocumentMarkerController.h 2022-03-12 00:32:18 UTC (rev 291200)
+++ trunk/Source/WebCore/dom/DocumentMarkerController.h 2022-03-12 00:56:15 UTC (rev 291201)
@@ -42,6 +42,7 @@
struct SimpleRange;
enum class RemovePartiallyOverlappingMarker : bool { No, Yes };
+enum class FilterMarkerResult : bool { Keep, Remove };
class DocumentMarkerController {
WTF_MAKE_NONCOPYABLE(DocumentMarkerController); WTF_MAKE_FAST_ALLOCATED;
@@ -63,11 +64,9 @@
// remove the marker. If the argument is false, we will adjust the span of the marker so that it retains
// the portion that is outside of the range.
WEBCORE_EXPORT void removeMarkers(const SimpleRange&, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers(), RemovePartiallyOverlappingMarker = RemovePartiallyOverlappingMarker::No);
- void removeMarkers(Node&, OffsetRange, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers(), const Function<bool(const DocumentMarker&)>& filterFunction = nullptr, RemovePartiallyOverlappingMarker = RemovePartiallyOverlappingMarker::No);
+ void removeMarkers(Node&, OffsetRange, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers(), const Function<FilterMarkerResult(const DocumentMarker&)>& filterFunction = nullptr, RemovePartiallyOverlappingMarker = RemovePartiallyOverlappingMarker::No);
- // Return true from filterFunction to remove the marker.
- // FIXME: This would be clearer if the `filter` function were refactored to return a strongly typed enum.
- WEBCORE_EXPORT void filterMarkers(const SimpleRange&, const Function<bool(const DocumentMarker&)>& filterFunction, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers(), RemovePartiallyOverlappingMarker = RemovePartiallyOverlappingMarker::No);
+ WEBCORE_EXPORT void filterMarkers(const SimpleRange&, const Function<FilterMarkerResult(const DocumentMarker&)>& filterFunction, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers(), RemovePartiallyOverlappingMarker = RemovePartiallyOverlappingMarker::No);
WEBCORE_EXPORT void removeMarkers(OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers());
void removeMarkers(Node&, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers());
Modified: trunk/Source/WebKit/ChangeLog (291200 => 291201)
--- trunk/Source/WebKit/ChangeLog 2022-03-12 00:32:18 UTC (rev 291200)
+++ trunk/Source/WebKit/ChangeLog 2022-03-12 00:56:15 UTC (rev 291201)
@@ -1,3 +1,18 @@
+2022-03-11 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Refactor the filter block in DocumentMarkerController::filterMarkers() to return an enum type
+ https://bugs.webkit.org/show_bug.cgi?id=237794
+
+ Reviewed by Megan Gardner.
+
+ Drive-by fix: only remove the spellchecking document marker in `removeAnnotationRelativeToSelection` if the
+ annotation string of the marker matches the given annotation string to the method.
+
+ * WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:
+ (WebKit::TextCheckingControllerProxy::removeAnnotationRelativeToSelection):
+ * WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
+ (WebKit::WebPage::clearDictationAlternatives):
+
2022-03-11 Alex Christensen <achristen...@webkit.org>
Finish implementing modify-headers actions for WKContentRuleList SPI
Modified: trunk/Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm (291200 => 291201)
--- trunk/Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm 2022-03-12 00:32:18 UTC (rev 291200)
+++ trunk/Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm 2022-03-12 00:56:15 UTC (rev 291201)
@@ -158,9 +158,8 @@
RefPtr document = CheckedRef(m_page.corePage()->focusController())->focusedOrMainFrame().document();
document->markers().filterMarkers(rangeAndOffset->range, [&] (const DocumentMarker& marker) {
if (!std::holds_alternative<WebCore::DocumentMarker::PlatformTextCheckingData>(marker.data()))
- return false;
- // FIXME: Is this return value flipped?
- return std::get<WebCore::DocumentMarker::PlatformTextCheckingData>(marker.data()).key != annotation;
+ return FilterMarkerResult::Keep;
+ return std::get<WebCore::DocumentMarker::PlatformTextCheckingData>(marker.data()).key == annotation ? FilterMarkerResult::Remove : FilterMarkerResult::Keep;
}, types);
}
Modified: trunk/Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm (291200 => 291201)
--- trunk/Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm 2022-03-12 00:32:18 UTC (rev 291200)
+++ trunk/Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm 2022-03-12 00:56:15 UTC (rev 291201)
@@ -326,9 +326,8 @@
auto documentRange = makeRangeSelectingNodeContents(*document);
document->markers().filterMarkers(documentRange, [&] (auto& marker) {
if (!std::holds_alternative<DocumentMarker::DictationData>(marker.data()))
- return false;
-
- return setOfContextsToRemove.contains(std::get<WebCore::DocumentMarker::DictationData>(marker.data()).context);
+ return FilterMarkerResult::Keep;
+ return setOfContextsToRemove.contains(std::get<WebCore::DocumentMarker::DictationData>(marker.data()).context) ? FilterMarkerResult::Remove : FilterMarkerResult::Keep;
}, DocumentMarker::DictationAlternatives);
}
