- Revision
- 188443
- Author
- [email protected]
- Date
- 2015-08-13 23:31:24 -0700 (Thu, 13 Aug 2015)
Log Message
Performing a Lookup on wrapped text puts the popover arrow in the wrong place (off to the right)
https://bugs.webkit.org/show_bug.cgi?id=148012
<rdar://problem/19238094>
Reviewed by Simon Fraser.
* platform/spi/mac/LookupSPI.h:
Add some SPI.
* WebView/WebView.mm:
(-[WebView _animationControllerForDictionaryLookupPopupInfo:]):
(-[WebView _showDictionaryLookupPopup:]):
Adopt the new SPI, handing it the first text rect, instead of having it
guess where to put the popover.
Also, null-check the TextIndicator.
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::didPerformDictionaryLookup):
* UIProcess/mac/WKImmediateActionController.mm:
(-[WKImmediateActionController _animationControllerForText]):
Adopt the new SPI, handing it the first text rect, instead of having it
guess where to put the popover.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (188442 => 188443)
--- trunk/Source/WebCore/ChangeLog 2015-08-14 06:21:03 UTC (rev 188442)
+++ trunk/Source/WebCore/ChangeLog 2015-08-14 06:31:24 UTC (rev 188443)
@@ -1,3 +1,14 @@
+2015-08-13 Tim Horton <[email protected]>
+
+ Performing a Lookup on wrapped text puts the popover arrow in the wrong place (off to the right)
+ https://bugs.webkit.org/show_bug.cgi?id=148012
+ <rdar://problem/19238094>
+
+ Reviewed by Simon Fraser.
+
+ * platform/spi/mac/LookupSPI.h:
+ Add some SPI.
+
2015-08-13 Simon Fraser <[email protected]>
Another Windows build fix.
Modified: trunk/Source/WebCore/platform/spi/mac/LookupSPI.h (188442 => 188443)
--- trunk/Source/WebCore/platform/spi/mac/LookupSPI.h 2015-08-14 06:21:03 UTC (rev 188442)
+++ trunk/Source/WebCore/platform/spi/mac/LookupSPI.h 2015-08-14 06:31:24 UTC (rev 188443)
@@ -42,6 +42,7 @@
+ (NSRange)tokenRangeForString:(NSString *)string range:(NSRange)range options:(NSDictionary **)options;
+ (void)showDefinitionForTerm:(NSAttributedString *)term atLocation:(NSPoint)screenPoint options:(NSDictionary *)options;
++ (void)showDefinitionForTerm:(NSAttributedString *)term relativeToRect:(NSRect)positioningRect ofView:(NSView *)positioningView options:(NSDictionary *)options;
+ (void)hideDefinition;
@end
@@ -50,10 +51,12 @@
#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
-@interface LULookupDefinitionModule (AnimationController)
+@interface LULookupDefinitionModule ()
+ (id<NSImmediateActionAnimationController>)lookupAnimationControllerForTerm:(NSAttributedString *)term atLocation:(NSPoint)screenPoint options:(NSDictionary *)options;
++ (id<NSImmediateActionAnimationController>)lookupAnimationControllerForTerm:(NSAttributedString *)term relativeToRect:(NSRect)positioningRect ofView:(NSView *)positioningView options:(NSDictionary *)options;
+
@end
#endif // __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
Modified: trunk/Source/WebKit/mac/ChangeLog (188442 => 188443)
--- trunk/Source/WebKit/mac/ChangeLog 2015-08-14 06:21:03 UTC (rev 188442)
+++ trunk/Source/WebKit/mac/ChangeLog 2015-08-14 06:31:24 UTC (rev 188443)
@@ -1,5 +1,20 @@
2015-08-13 Tim Horton <[email protected]>
+ Performing a Lookup on wrapped text puts the popover arrow in the wrong place (off to the right)
+ https://bugs.webkit.org/show_bug.cgi?id=148012
+ <rdar://problem/19238094>
+
+ Reviewed by Simon Fraser.
+
+ * WebView/WebView.mm:
+ (-[WebView _animationControllerForDictionaryLookupPopupInfo:]):
+ (-[WebView _showDictionaryLookupPopup:]):
+ Adopt the new SPI, handing it the first text rect, instead of having it
+ guess where to put the popover.
+ Also, null-check the TextIndicator.
+
+2015-08-13 Tim Horton <[email protected]>
+
Refactor and improve TextIndicator to prepare for tests
https://bugs.webkit.org/show_bug.cgi?id=147622
Modified: trunk/Source/WebKit/mac/WebView/WebView.mm (188442 => 188443)
--- trunk/Source/WebKit/mac/WebView/WebView.mm 2015-08-14 06:21:03 UTC (rev 188442)
+++ trunk/Source/WebKit/mac/WebView/WebView.mm 2015-08-14 06:31:24 UTC (rev 188443)
@@ -8551,12 +8551,7 @@
if (!dictionaryPopupInfo.attributedString)
return nil;
- NSPoint textBaselineOrigin = dictionaryPopupInfo.origin;
-
- // Convert to screen coordinates.
- textBaselineOrigin = [self.window convertRectToScreen:NSMakeRect(textBaselineOrigin.x, textBaselineOrigin.y, 0, 0)].origin;
-
- if (canLoadLUTermOptionDisableSearchTermIndicator() && canLoadLUNotificationPopoverWillClose()) {
+ if (canLoadLUTermOptionDisableSearchTermIndicator() && canLoadLUNotificationPopoverWillClose() && dictionaryPopupInfo.textIndicator) {
if (!_private->hasInitializedLookupObserver) {
[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(_dictionaryLookupPopoverWillClose:) name:getLUNotificationPopoverWillClose() object:nil];
_private->hasInitializedLookupObserver = YES;
@@ -8567,9 +8562,17 @@
mutableOptions = adoptNS([[NSMutableDictionary alloc] init]);
[mutableOptions setObject:@YES forKey:getLUTermOptionDisableSearchTermIndicator()];
[self _setTextIndicator:*dictionaryPopupInfo.textIndicator withLifetime:TextIndicatorWindowLifetime::Permanent];
- return [getLULookupDefinitionModuleClass() lookupAnimationControllerForTerm:dictionaryPopupInfo.attributedString.get() atLocation:textBaselineOrigin options:mutableOptions.get()];
+
+ if ([getLULookupDefinitionModuleClass() respondsToSelector:@selector(lookupAnimationControllerForTerm:relativeToRect:ofView:options:)]) {
+ FloatRect firstTextRectInViewCoordinates = dictionaryPopupInfo.textIndicator->textRectsInBoundingRectCoordinates()[0];
+ firstTextRectInViewCoordinates.moveBy(dictionaryPopupInfo.textIndicator->textBoundingRectInRootViewCoordinates().location());
+ return [getLULookupDefinitionModuleClass() lookupAnimationControllerForTerm:dictionaryPopupInfo.attributedString.get() relativeToRect:[self _convertRectFromRootView:firstTextRectInViewCoordinates] ofView:self options:mutableOptions.get()];
+ }
}
+ // Convert to screen coordinates.
+ NSPoint textBaselineOrigin = [self.window convertRectToScreen:NSMakeRect(dictionaryPopupInfo.origin.x, dictionaryPopupInfo.origin.y, 0, 0)].origin;
+
return [getLULookupDefinitionModuleClass() lookupAnimationControllerForTerm:dictionaryPopupInfo.attributedString.get() atLocation:textBaselineOrigin options:dictionaryPopupInfo.options.get()];
}
#endif // __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
@@ -8617,12 +8620,7 @@
if (!dictionaryPopupInfo.attributedString)
return;
- NSPoint textBaselineOrigin = dictionaryPopupInfo.origin;
-
- // Convert to screen coordinates.
- textBaselineOrigin = [self.window convertRectToScreen:NSMakeRect(textBaselineOrigin.x, textBaselineOrigin.y, 0, 0)].origin;
-
- if (canLoadLUTermOptionDisableSearchTermIndicator() && canLoadLUNotificationPopoverWillClose()) {
+ if (canLoadLUTermOptionDisableSearchTermIndicator() && canLoadLUNotificationPopoverWillClose() && dictionaryPopupInfo.textIndicator) {
if (!_private->hasInitializedLookupObserver) {
[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(_dictionaryLookupPopoverWillClose:) name:getLUNotificationPopoverWillClose() object:nil];
_private->hasInitializedLookupObserver = YES;
@@ -8633,9 +8631,19 @@
mutableOptions = adoptNS([[NSMutableDictionary alloc] init]);
[mutableOptions setObject:@YES forKey:getLUTermOptionDisableSearchTermIndicator()];
[self _setTextIndicator:*dictionaryPopupInfo.textIndicator withLifetime:TextIndicatorWindowLifetime::Permanent];
- [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.get() atLocation:textBaselineOrigin options:mutableOptions.get()];
- } else
- [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.get() atLocation:textBaselineOrigin options:dictionaryPopupInfo.options.get()];
+
+ if ([getLULookupDefinitionModuleClass() respondsToSelector:@selector(showDefinitionForTerm:relativeToRect:ofView:options:)]) {
+ FloatRect firstTextRectInViewCoordinates = dictionaryPopupInfo.textIndicator->textRectsInBoundingRectCoordinates()[0];
+ firstTextRectInViewCoordinates.moveBy(dictionaryPopupInfo.textIndicator->textBoundingRectInRootViewCoordinates().location());
+ [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.get() relativeToRect:[self _convertRectFromRootView:firstTextRectInViewCoordinates] ofView:self options:mutableOptions.get()];
+ return;
+ }
+ }
+
+ // Convert to screen coordinates.
+ NSPoint textBaselineOrigin = [self.window convertRectToScreen:NSMakeRect(dictionaryPopupInfo.origin.x, dictionaryPopupInfo.origin.y, 0, 0)].origin;
+
+ [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.get() atLocation:textBaselineOrigin options:dictionaryPopupInfo.options.get()];
}
- (void)_dictionaryLookupPopoverWillClose:(NSNotification *)notification
Modified: trunk/Source/WebKit2/ChangeLog (188442 => 188443)
--- trunk/Source/WebKit2/ChangeLog 2015-08-14 06:21:03 UTC (rev 188442)
+++ trunk/Source/WebKit2/ChangeLog 2015-08-14 06:31:24 UTC (rev 188443)
@@ -1,3 +1,18 @@
+2015-08-13 Tim Horton <[email protected]>
+
+ Performing a Lookup on wrapped text puts the popover arrow in the wrong place (off to the right)
+ https://bugs.webkit.org/show_bug.cgi?id=148012
+ <rdar://problem/19238094>
+
+ Reviewed by Simon Fraser.
+
+ * UIProcess/mac/PageClientImpl.mm:
+ (WebKit::PageClientImpl::didPerformDictionaryLookup):
+ * UIProcess/mac/WKImmediateActionController.mm:
+ (-[WKImmediateActionController _animationControllerForText]):
+ Adopt the new SPI, handing it the first text rect, instead of having it
+ guess where to put the popover.
+
2015-08-13 Commit Queue <[email protected]>
Unreviewed, rolling out r188418.
Modified: trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm (188442 => 188443)
--- trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm 2015-08-14 06:21:03 UTC (rev 188442)
+++ trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm 2015-08-14 06:31:24 UTC (rev 188443)
@@ -588,12 +588,6 @@
if (!getLULookupDefinitionModuleClass())
return;
- NSPoint textBaselineOrigin = dictionaryPopupInfo.origin;
-
- // Convert to screen coordinates.
- textBaselineOrigin = [m_wkView convertPoint:textBaselineOrigin toView:nil];
- textBaselineOrigin = [m_wkView.window convertRectToScreen:NSMakeRect(textBaselineOrigin.x, textBaselineOrigin.y, 0, 0)].origin;
-
RetainPtr<NSMutableDictionary> mutableOptions = adoptNS([(NSDictionary *)dictionaryPopupInfo.options.get() mutableCopy]);
[m_wkView _prepareForDictionaryLookup];
@@ -601,9 +595,22 @@
if (canLoadLUTermOptionDisableSearchTermIndicator() && dictionaryPopupInfo.textIndicator.contentImage) {
[m_wkView _setTextIndicator:TextIndicator::create(dictionaryPopupInfo.textIndicator) withLifetime:TextIndicatorWindowLifetime::Permanent];
[mutableOptions setObject:@YES forKey:getLUTermOptionDisableSearchTermIndicator()];
- [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.string.get() atLocation:textBaselineOrigin options:mutableOptions.get()];
- } else
- [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.string.get() atLocation:textBaselineOrigin options:mutableOptions.get()];
+
+ if ([getLULookupDefinitionModuleClass() respondsToSelector:@selector(showDefinitionForTerm:relativeToRect:ofView:options:)]) {
+ FloatRect firstTextRectInViewCoordinates = dictionaryPopupInfo.textIndicator.textRectsInBoundingRectCoordinates[0];
+ firstTextRectInViewCoordinates.moveBy(dictionaryPopupInfo.textIndicator.textBoundingRectInRootViewCoordinates.location());
+ [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.string.get() relativeToRect:firstTextRectInViewCoordinates ofView:m_wkView options:mutableOptions.get()];
+ return;
+ }
+ }
+
+ NSPoint textBaselineOrigin = dictionaryPopupInfo.origin;
+
+ // Convert to screen coordinates.
+ textBaselineOrigin = [m_wkView convertPoint:textBaselineOrigin toView:nil];
+ textBaselineOrigin = [m_wkView.window convertRectToScreen:NSMakeRect(textBaselineOrigin.x, textBaselineOrigin.y, 0, 0)].origin;
+
+ [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.string.get() atLocation:textBaselineOrigin options:mutableOptions.get()];
}
void PageClientImpl::dismissContentRelativeChildWindows(bool withAnimation)
Modified: trunk/Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm (188442 => 188443)
--- trunk/Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm 2015-08-14 06:21:03 UTC (rev 188442)
+++ trunk/Source/WebKit2/UIProcess/mac/WKImmediateActionController.mm 2015-08-14 06:31:24 UTC (rev 188443)
@@ -470,18 +470,23 @@
[_wkView _prepareForDictionaryLookup];
+ RetainPtr<NSMutableDictionary> mutableOptions = adoptNS([(NSDictionary *)dictionaryPopupInfo.options.get() mutableCopy]);
+ if (canLoadLUTermOptionDisableSearchTermIndicator() && dictionaryPopupInfo.textIndicator.contentImage) {
+ [_wkView _setTextIndicator:TextIndicator::create(dictionaryPopupInfo.textIndicator) withLifetime:TextIndicatorWindowLifetime::Permanent];
+ [mutableOptions setObject:@YES forKey:getLUTermOptionDisableSearchTermIndicator()];
+
+ if ([getLULookupDefinitionModuleClass() respondsToSelector:@selector(lookupAnimationControllerForTerm:relativeToRect:ofView:options:)]) {
+ FloatRect firstTextRectInViewCoordinates = dictionaryPopupInfo.textIndicator.textRectsInBoundingRectCoordinates[0];
+ firstTextRectInViewCoordinates.moveBy(dictionaryPopupInfo.textIndicator.textBoundingRectInRootViewCoordinates.location());
+ return [getLULookupDefinitionModuleClass() lookupAnimationControllerForTerm:dictionaryPopupInfo.attributedString.string.get() relativeToRect:firstTextRectInViewCoordinates ofView:_wkView options:mutableOptions.get()];
+ }
+ }
+
// Convert baseline to screen coordinates.
NSPoint textBaselineOrigin = dictionaryPopupInfo.origin;
textBaselineOrigin = [_wkView convertPoint:textBaselineOrigin toView:nil];
textBaselineOrigin = [_wkView.window convertRectToScreen:NSMakeRect(textBaselineOrigin.x, textBaselineOrigin.y, 0, 0)].origin;
- RetainPtr<NSMutableDictionary> mutableOptions = adoptNS([(NSDictionary *)dictionaryPopupInfo.options.get() mutableCopy]);
- if (canLoadLUTermOptionDisableSearchTermIndicator() && dictionaryPopupInfo.textIndicator.contentImage) {
- RefPtr<TextIndicator> indicator = TextIndicator::create(dictionaryPopupInfo.textIndicator);
- [_wkView _setTextIndicator:*indicator withLifetime:TextIndicatorWindowLifetime::Permanent];
- [mutableOptions setObject:@YES forKey:getLUTermOptionDisableSearchTermIndicator()];
- return [getLULookupDefinitionModuleClass() lookupAnimationControllerForTerm:dictionaryPopupInfo.attributedString.string.get() atLocation:textBaselineOrigin options:mutableOptions.get()];
- }
return [getLULookupDefinitionModuleClass() lookupAnimationControllerForTerm:dictionaryPopupInfo.attributedString.string.get() atLocation:textBaselineOrigin options:mutableOptions.get()];
}
