- Revision
- 263531
- Author
- [email protected]
- Date
- 2020-06-25 15:03:21 -0700 (Thu, 25 Jun 2020)
Log Message
[iOS] -_requestTextInputContextsInRect cannot find empty Quip spreadsheet title
https://bugs.webkit.org/show_bug.cgi?id=213564
<rdar://problem/59355847>
Reviewed by Simon Fraser.
Source/WebCore:
Special case for a focused editable inline element that has no line box, which is
what the Quip spreadsheet title can become if its contents are deleted. The engine
optimizes to avoid creating lines and line boxes for empty inlines, hit testing,
and painting them. It's complicated to patch all this up with a 100% correct
solution without forcing more line box creation. So, just add a special case.
* page/Page.cpp:
(WebCore::Page::editableElementsInRect const): Add the root editable element of
the focused element to the result set if its rect intersects the search rect. Note
that its rect can be empty e.g. <span contenteditable="true"></span>. So, I use
a new function on FloatRect to perform the intersection test. I also updated the
ASSERT() to also use this new function. This was not necessary, but I did it to
future the proof this code should hit testing one day return empty inlines.
* platform/graphics/FloatRect.cpp:
(WebCore::FloatRect::inclusivelyIntersects const): Added.
(WebCore::FloatRect::intersects const): While I was in this file I fixed up a comment
in this function to be more precise.
* platform/graphics/FloatRect.h:
Source/WebKit:
Check if the search rect intersects the interaction rect, even if it's empty, so long as
there is an assisted element. If there isn't an assisted element just do what we do now.
Otherwise, if the interaction rect is empty then this means the focused element is an
empty inline and it's not tracked in the editable region (see WebCore ChangeLog entry for
more details). In this case, just ask the web process directly to find the contexts.
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _requestTextInputContextsInRect:completionHandler:]):
Tools:
Add some tests.
* TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (263530 => 263531)
--- trunk/Source/WebCore/ChangeLog 2020-06-25 21:19:49 UTC (rev 263530)
+++ trunk/Source/WebCore/ChangeLog 2020-06-25 22:03:21 UTC (rev 263531)
@@ -1,3 +1,30 @@
+2020-06-25 Daniel Bates <[email protected]>
+
+ [iOS] -_requestTextInputContextsInRect cannot find empty Quip spreadsheet title
+ https://bugs.webkit.org/show_bug.cgi?id=213564
+ <rdar://problem/59355847>
+
+ Reviewed by Simon Fraser.
+
+ Special case for a focused editable inline element that has no line box, which is
+ what the Quip spreadsheet title can become if its contents are deleted. The engine
+ optimizes to avoid creating lines and line boxes for empty inlines, hit testing,
+ and painting them. It's complicated to patch all this up with a 100% correct
+ solution without forcing more line box creation. So, just add a special case.
+
+ * page/Page.cpp:
+ (WebCore::Page::editableElementsInRect const): Add the root editable element of
+ the focused element to the result set if its rect intersects the search rect. Note
+ that its rect can be empty e.g. <span contenteditable="true"></span>. So, I use
+ a new function on FloatRect to perform the intersection test. I also updated the
+ ASSERT() to also use this new function. This was not necessary, but I did it to
+ future the proof this code should hit testing one day return empty inlines.
+ * platform/graphics/FloatRect.cpp:
+ (WebCore::FloatRect::inclusivelyIntersects const): Added.
+ (WebCore::FloatRect::intersects const): While I was in this file I fixed up a comment
+ in this function to be more precise.
+ * platform/graphics/FloatRect.h:
+
2020-06-25 Wenson Hsieh <[email protected]>
Unreviewed, fix the macOS 11 build after r263519
Modified: trunk/Source/WebCore/page/Page.cpp (263530 => 263531)
--- trunk/Source/WebCore/page/Page.cpp 2020-06-25 21:19:49 UTC (rev 263530)
+++ trunk/Source/WebCore/page/Page.cpp 2020-06-25 22:03:21 UTC (rev 263531)
@@ -949,10 +949,22 @@
auto& nodeSet = hitTestResult.listBasedTestResult();
for (auto& node : nodeSet) {
if (auto* editableElement = rootEditableElement(node)) {
- ASSERT(searchRectInRootViewCoordinates.intersects(editableElement->clientRect()));
+ ASSERT(searchRectInRootViewCoordinates.inclusivelyIntersects(editableElement->clientRect()));
rootEditableElements.add(*editableElement);
}
}
+
+ // Fix up for a now empty focused inline element, e.g. <span contenteditable='true'>Hello</span> became
+ // <span contenteditable='true'></span>. Hit testing will likely not find this element because the engine
+ // tries to avoid creating line boxes, which are things it hit tests, for them to reduce memory. If the
+ // focused element is inside the search rect it's the most likely target for future editing operations,
+ // even if it's empty. So, we special case it here.
+ if (auto* focusedElement = focusController().focusedOrMainFrame().document()->focusedElement()) {
+ if (searchRectInRootViewCoordinates.inclusivelyIntersects(focusedElement->clientRect())) {
+ if (auto* editableElement = rootEditableElement(*focusedElement))
+ rootEditableElements.add(*editableElement);
+ }
+ }
return WTF::map(rootEditableElements, [](const auto& element) { return element.copyRef(); });
}
Modified: trunk/Source/WebCore/platform/graphics/FloatRect.cpp (263530 => 263531)
--- trunk/Source/WebCore/platform/graphics/FloatRect.cpp 2020-06-25 21:19:49 UTC (rev 263530)
+++ trunk/Source/WebCore/platform/graphics/FloatRect.cpp 2020-06-25 22:03:21 UTC (rev 263531)
@@ -54,9 +54,15 @@
&& isWithinIntRange(maxX()) && isWithinIntRange(maxY());
}
+bool FloatRect::inclusivelyIntersects(const FloatRect& other) const
+{
+ return width() >= 0 && height() >= 0 && other.width() >= 0 && other.height() >= 0
+ && x() <= other.maxX() && other.x() <= maxX() && y() <= other.maxY() && other.y() <= maxY();
+}
+
bool FloatRect::intersects(const FloatRect& other) const
{
- // Checking emptiness handles negative widths as well as zero.
+ // Checking emptiness handles negative widths and heights as well as zero.
return !isEmpty() && !other.isEmpty()
&& x() < other.maxX() && other.x() < maxX()
&& y() < other.maxY() && other.y() < maxY();
Modified: trunk/Source/WebCore/platform/graphics/FloatRect.h (263530 => 263531)
--- trunk/Source/WebCore/platform/graphics/FloatRect.h 2020-06-25 21:19:49 UTC (rev 263530)
+++ trunk/Source/WebCore/platform/graphics/FloatRect.h 2020-06-25 22:03:21 UTC (rev 263531)
@@ -162,6 +162,7 @@
FloatPoint maxXMaxYCorner() const { return FloatPoint(m_location.x() + m_size.width(), m_location.y() + m_size.height()); } // typically bottomRight
WEBCORE_EXPORT bool intersects(const FloatRect&) const;
+ WEBCORE_EXPORT bool inclusivelyIntersects(const FloatRect&) const;
WEBCORE_EXPORT bool contains(const FloatRect&) const;
WEBCORE_EXPORT bool contains(const FloatPoint&, ContainsMode = InsideOrOnStroke) const;
Modified: trunk/Source/WebKit/ChangeLog (263530 => 263531)
--- trunk/Source/WebKit/ChangeLog 2020-06-25 21:19:49 UTC (rev 263530)
+++ trunk/Source/WebKit/ChangeLog 2020-06-25 22:03:21 UTC (rev 263531)
@@ -1,3 +1,20 @@
+2020-06-25 Daniel Bates <[email protected]>
+
+ [iOS] -_requestTextInputContextsInRect cannot find empty Quip spreadsheet title
+ https://bugs.webkit.org/show_bug.cgi?id=213564
+ <rdar://problem/59355847>
+
+ Reviewed by Simon Fraser.
+
+ Check if the search rect intersects the interaction rect, even if it's empty, so long as
+ there is an assisted element. If there isn't an assisted element just do what we do now.
+ Otherwise, if the interaction rect is empty then this means the focused element is an
+ empty inline and it's not tracked in the editable region (see WebCore ChangeLog entry for
+ more details). In this case, just ask the web process directly to find the contexts.
+
+ * UIProcess/ios/WKContentViewInteraction.mm:
+ (-[WKContentView _requestTextInputContextsInRect:completionHandler:]):
+
2020-06-25 Kate Cheney <[email protected]>
Allow service workers for web browsers
Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (263530 => 263531)
--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2020-06-25 21:19:49 UTC (rev 263530)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm 2020-06-25 22:03:21 UTC (rev 263531)
@@ -5198,13 +5198,15 @@
- (void)_requestTextInputContextsInRect:(CGRect)rect completionHandler:(void (^)(NSArray<_WKTextInputContext *> *))completionHandler
{
+ WebCore::FloatRect searchRect { rect };
#if ENABLE(EDITABLE_REGION)
- if (!self.webView._editable && !WebKit::mayContainEditableElementsInRect(self, rect)) {
+ bool hitInteractionRect = self._hasFocusedElement && searchRect.inclusivelyIntersects(_focusedElementInformation.interactionRect);
+ if (!self.webView._editable && !hitInteractionRect && !WebKit::mayContainEditableElementsInRect(self, rect)) {
completionHandler(@[ ]);
return;
}
#endif
- _page->textInputContextsInRect(rect, [weakSelf = WeakObjCPtr<WKContentView>(self), completionHandler = makeBlockPtr(completionHandler)] (const Vector<WebCore::ElementContext>& contexts) {
+ _page->textInputContextsInRect(searchRect, [weakSelf = WeakObjCPtr<WKContentView>(self), completionHandler = makeBlockPtr(completionHandler)] (const Vector<WebCore::ElementContext>& contexts) {
auto strongSelf = weakSelf.get();
if (!strongSelf || contexts.isEmpty()) {
completionHandler(@[ ]);
Modified: trunk/Tools/ChangeLog (263530 => 263531)
--- trunk/Tools/ChangeLog 2020-06-25 21:19:49 UTC (rev 263530)
+++ trunk/Tools/ChangeLog 2020-06-25 22:03:21 UTC (rev 263531)
@@ -1,3 +1,16 @@
+2020-06-25 Daniel Bates <[email protected]>
+
+ [iOS] -_requestTextInputContextsInRect cannot find empty Quip spreadsheet title
+ https://bugs.webkit.org/show_bug.cgi?id=213564
+ <rdar://problem/59355847>
+
+ Reviewed by Simon Fraser.
+
+ Add some tests.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm:
+ (TestWebKitAPI::TEST):
+
2020-06-25 Sam Weinig <[email protected]>
Add a new templated string type to help write more idiomatic parsing code
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm (263530 => 263531)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm 2020-06-25 21:19:49 UTC (rev 263530)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RequestTextInputContext.mm 2020-06-25 22:03:21 UTC (rev 263531)
@@ -330,6 +330,40 @@
EXPECT_EQ(0UL, [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]].count);
}
+TEST(RequestTextInputContext, FocusedEditableEmptyLine)
+{
+ RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ RetainPtr<TestWKWebView> webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+ auto inputDelegate = adoptNS([[TestInputDelegate alloc] init]);
+ [inputDelegate setFocusStartsInputSessionPolicyHandler:[] (WKWebView *, id <_WKFocusedElementInfo>) { return _WKFocusStartsInputSessionPolicyAllow; }];
+ [webView _setInputDelegate:inputDelegate.get()];
+
+ webViewLoadHTMLStringAndWaitForAllFramesToPaint(webView.get(), applyStyle(@"<span id='span' contenteditable='true'></span>"));
+ [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"span.focus()"];
+ EXPECT_WK_STREQ("SPAN", [webView stringByEvaluatingJavaScript:@"document.activeElement.tagName"]);
+
+ NSArray<_WKTextInputContext *> *contexts = [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]];
+ EXPECT_EQ(1UL, contexts.count);
+ EXPECT_EQ(CGRectMake(0, 0, 0, 0), contexts[0].boundingRect);
+}
+
+TEST(RequestTextInputContext, FocusedEditableLineWithOnlyLineBreak)
+{
+ RetainPtr<WKWebViewConfiguration> configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+ RetainPtr<TestWKWebView> webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+ auto inputDelegate = adoptNS([[TestInputDelegate alloc] init]);
+ [inputDelegate setFocusStartsInputSessionPolicyHandler:[] (WKWebView *, id <_WKFocusedElementInfo>) { return _WKFocusStartsInputSessionPolicyAllow; }];
+ [webView _setInputDelegate:inputDelegate.get()];
+
+ webViewLoadHTMLStringAndWaitForAllFramesToPaint(webView.get(), applyStyle(@"<span id='span' contenteditable='true'><br></span>"));
+ [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"span.focus()"];
+ EXPECT_WK_STREQ("SPAN", [webView stringByEvaluatingJavaScript:@"document.activeElement.tagName"]);
+
+ NSArray<_WKTextInputContext *> *contexts = [webView synchronouslyRequestTextInputContextsInRect:[webView bounds]];
+ EXPECT_EQ(1UL, contexts.count);
+ EXPECT_EQ(CGRectMake(0, 0, 0, 19), contexts[0].boundingRect);
+}
+
TEST(RequestTextInputContext, FocusAfterNavigation)
{
auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
