Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: d34e00e6207ce86589cf77dd2f91a1d905fddd3f
https://github.com/WebKit/WebKit/commit/d34e00e6207ce86589cf77dd2f91a1d905fddd3f
Author: Wenson Hsieh <[email protected]>
Date: 2022-10-17 (Mon, 17 Oct 2022)
Changed paths:
M Source/WebCore/PlatformMac.cmake
M Source/WebCore/SourcesCocoa.txt
M Source/WebCore/WebCore.xcodeproj/project.pbxproj
M Source/WebCore/editing/cocoa/DictionaryLookup.mm
M Source/WebCore/editing/mac/DictionaryLookup.h
A Source/WebCore/platform/mac/RevealUtilities.h
A Source/WebCore/platform/mac/RevealUtilities.mm
M Source/WebKit/Platform/mac/MenuUtilities.mm
M Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig
M Tools/TestWebKitAPI/Tests/mac/ImmediateActionTests.mm
Log Message:
-----------
REGRESSION (254359@main): Yellow highlight for lookup doesn't show text
https://bugs.webkit.org/show_bug.cgi?id=246586
rdar://100828328
Reviewed by Aditya Keerthi.
After 254359@main, we no longer have a retain cycle between the
`WebRevealHighlight` and the
`_clearTextIndicator` callback block data. However, this means that nothing
currently holds on to
`WebRevealHighlight` after we return from
`showPopupOrCreateAnimationController`, which causes the
text indicator snapshot and data to not show up when looking up a word (e.g.
via force click).
To fix this without reintroducing the retain cycle, we instead tie the lifetime
of the
`WebRevealHighlight` to the `RVPresentingContext` itself, which only has a weak
pointer back to the
reveal highlight (via `-highlightDelegate`). This ensures that the highlight
information will never
prematurely disappear from underneath the presenting context.
Note that this technique is already used in `MenuUtilities.mm` inside
`menuForTelephoneNumber`,
which handles context menu presentation when force clicking on a data detected
phone number. To
avoid repeating this code in both places, we move it into a new helper function
in `RevealUtilities`
and call it from both places.
See below for more details.
Test: ImmediateActionTests.ImmediateActionOverText
* Source/WebCore/PlatformMac.cmake:
* Source/WebCore/SourcesCocoa.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/editing/cocoa/DictionaryLookup.mm:
(-[WebRevealHighlight
initWithHighlightRect:useDefaultHighlight:attributedString:clearTextIndicatorCallback:]):
Drive-by refactoring: remove `setClearTextIndicator:`, and just pass it in when
creating the reveal
highlight. Currently, this method is only called right after initialization
anyways.
(WebCore::showPopupOrCreateAnimationController):
Use the new `createRVPresentingContextWithRetainedDelegate` method to create
the presenting context.
(-[WebRevealHighlight
initWithHighlightRect:useDefaultHighlight:attributedString:]): Deleted.
(-[WebRevealHighlight setClearTextIndicator:]): Deleted.
* Source/WebCore/editing/mac/DictionaryLookup.h:
* Source/WebCore/platform/mac/RevealUtilities.h: Added.
* Source/WebCore/platform/mac/RevealUtilities.mm: Added.
Add a new file that contains a helper method
(`createRVPresentingContextWithRetainedDelegate`) that
allocates a new `RVPresentingContext` with the given parameters, and ties the
lifetime of the
delegate to the context.
(WebCore::createRVPresentingContextWithRetainedDelegate):
* Source/WebKit/Platform/mac/MenuUtilities.mm:
(WebKit::menuForTelephoneNumber):
Use the new helper method here, too.
* Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig:
Additionally link against Reveal in TestWebKitAPI on macOS, so that it's
simpler to intercept and
replace the call to `-initWithPointerLocationInView:inView:highlightDelegate:`.
* Tools/TestWebKitAPI/Tests/mac/ImmediateActionTests.mm:
(-[NSObject swizzled_initWithPointerLocationInView:inView:highlightDelegate:]):
(TestWebKitAPI::swizzlePresentingContextInitialization):
Augment a test to verify that the `RVPresentingContext`'s highlight delegate
doesn't vanish right
after triggering Look Up via a force click.
Canonical link: https://commits.webkit.org/255642@main
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes