Skip to content

Commit

Permalink
[iOS 17] Correction indicators persist after typing
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=258954
rdar://111764235

Reviewed by Wenson Hsieh.

When upstreaming the autocorrection enhancements implementation, the code to dismiss the markers
was accidentally put inside a compile time guard which was only true for `PLATFORM(MAC)` and not iOS.

Fix by moving the code outside of the compile time guard.

Also refactors some methods in `Internals.cpp` to reduce duplication.

* Source/WebCore/editing/AlternativeTextController.cpp:
(WebCore::removeAutocorrectionIndictorMarkers):
(WebCore::AlternativeTextController::respondToAppliedEditing):
(WebCore::removeCorrectionIndicatorMarkers): Deleted.
* Source/WebCore/editing/AlternativeTextController.h:
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::hasMarkerFor):
(WebCore::Internals::hasSpellingMarker):
(WebCore::Internals::hasGrammarMarker):
(WebCore::Internals::hasAutocorrectedMarker):
(WebCore::Internals::hasDictationAlternativesMarker):
(WebCore::Internals::hasCorrectionIndicatorMarker):
* Source/WebCore/testing/Internals.h:
* Source/WebCore/testing/Internals.idl:
* Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h:
* Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm:
(-[WKWebView applyAutocorrection:toString:isCandidate:withCompletionHandler:]):
* Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm:
(TEST):

Canonical link: https://commits.webkit.org/265854@main
  • Loading branch information
rr-codes committed Jul 7, 2023
1 parent 0de1928 commit cafa71f
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 52 deletions.
54 changes: 27 additions & 27 deletions Source/WebCore/editing/AlternativeTextController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -419,33 +419,6 @@ void AlternativeTextController::respondToChangedSelection(const VisibleSelection
}
}

static inline void removeCorrectionIndicatorMarkers(Document& document)
{
#if HAVE(AUTOCORRECTION_ENHANCEMENTS)
document.markers().dismissMarkers(DocumentMarker::CorrectionIndicator);
#else
document.markers().removeMarkers(DocumentMarker::CorrectionIndicator);
#endif
}

void AlternativeTextController::respondToAppliedEditing(Document& document, CompositeEditCommand* command)
{
if (command->isTopLevelCommand() && !command->shouldRetainAutocorrectionIndicator())
removeCorrectionIndicatorMarkers(m_document);

markPrecedingWhitespaceForDeletedAutocorrectionAfterCommand(command);
m_originalStringForLastDeletedAutocorrection = String();

dismiss(ReasonForDismissingAlternativeText::Ignored);

#if HAVE(AUTOCORRECTION_ENHANCEMENTS) && PLATFORM(IOS_FAMILY)
if (!command->shouldRetainAutocorrectionIndicator())
document.markers().dismissMarkers(DocumentMarker::CorrectionIndicator);
#else
UNUSED_PARAM(document);
#endif
}

void AlternativeTextController::respondToUnappliedEditing(EditCommandComposition* command)
{
if (!command->wasCreateLinkCommand())
Expand Down Expand Up @@ -674,7 +647,34 @@ void AlternativeTextController::applyAlternativeTextToRange(const SimpleRange& r
addMarker(replacementRange, markerType, markerDescriptionForAppliedAlternativeText(alternativeType, markerType));
}

#endif // USE(AUTOCORRECTION_PANEL)

void AlternativeTextController::removeCorrectionIndicatorMarkers()
{
#if HAVE(AUTOCORRECTION_ENHANCEMENTS)
m_document.markers().dismissMarkers(DocumentMarker::CorrectionIndicator);
#else
m_document.markers().removeMarkers(DocumentMarker::CorrectionIndicator);
#endif
}

void AlternativeTextController::respondToAppliedEditing(CompositeEditCommand* command)
{
#if USE(AUTOCORRECTION_PANEL)
if (command->isTopLevelCommand() && !command->shouldRetainAutocorrectionIndicator())
removeCorrectionIndicatorMarkers();

markPrecedingWhitespaceForDeletedAutocorrectionAfterCommand(command);
m_originalStringForLastDeletedAutocorrection = String();

dismiss(ReasonForDismissingAlternativeText::Ignored);
#elif HAVE(AUTOCORRECTION_ENHANCEMENTS) && PLATFORM(IOS_FAMILY)
if (!command->shouldRetainAutocorrectionIndicator())
removeCorrectionIndicatorMarkers();
#else
UNUSED_PARAM(command);
#endif
}

bool AlternativeTextController::insertDictatedText(const String& text, const Vector<DictationAlternative>& dictationAlternatives, Event* triggeringEvent)
{
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/editing/AlternativeTextController.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ class AlternativeTextController {
bool applyAutocorrectionBeforeTypingIfAppropriate() UNLESS_ENABLED({ return false; })

void respondToUnappliedSpellCorrection(const VisibleSelection&, const String& corrected, const String& correction) UNLESS_ENABLED({ UNUSED_PARAM(corrected); UNUSED_PARAM(correction); })
void respondToAppliedEditing(Document&, CompositeEditCommand*) UNLESS_ENABLED({ })
void respondToAppliedEditing(CompositeEditCommand*);
void respondToUnappliedEditing(EditCommandComposition*) UNLESS_ENABLED({ })
void respondToChangedSelection(const VisibleSelection& oldSelection) UNLESS_ENABLED({ UNUSED_PARAM(oldSelection); })

Expand Down Expand Up @@ -136,6 +136,8 @@ class AlternativeTextController {
AlternativeTextClient* alternativeTextClient();
#endif

void removeCorrectionIndicatorMarkers();

Document& m_document;
};

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/editing/Editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1249,7 +1249,7 @@ void Editor::appliedEditing(CompositeEditCommand& command)
if (command.isTopLevelCommand()) {
updateEditorUINowIfScheduled();

m_alternativeTextController->respondToAppliedEditing(m_document, &command);
m_alternativeTextController->respondToAppliedEditing(&command);

if (!command.preservesTypingStyle())
m_document.selection().clearTypingStyle();
Expand Down
40 changes: 17 additions & 23 deletions Source/WebCore/testing/Internals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2642,37 +2642,40 @@ void Internals::updateEditorUINowIfScheduled()
}
}

bool Internals::hasSpellingMarker(int from, int length)
bool Internals::hasMarkerFor(DocumentMarker::MarkerType type, int from, int length)
{
Document* document = contextDocument();
if (!document || !document->frame())
return false;

updateEditorUINowIfScheduled();

return document->editor().selectionStartHasMarkerFor(DocumentMarker::Spelling, from, length);
return document->editor().selectionStartHasMarkerFor(type, from, length);
}

bool Internals::hasAutocorrectedMarker(int from, int length)
bool Internals::hasSpellingMarker(int from, int length)
{
Document* document = contextDocument();
if (!document || !document->frame())
return false;
return hasMarkerFor(DocumentMarker::Spelling, from, length);
}

updateEditorUINowIfScheduled();
bool Internals::hasGrammarMarker(int from, int length)
{
return hasMarkerFor(DocumentMarker::Grammar, from, length);
}

return document->editor().selectionStartHasMarkerFor(DocumentMarker::Autocorrected, from, length);
bool Internals::hasAutocorrectedMarker(int from, int length)
{
return hasMarkerFor(DocumentMarker::Autocorrected, from, length);
}

bool Internals::hasDictationAlternativesMarker(int from, int length)
{
auto* document = contextDocument();
if (!document || !document->frame())
return false;

updateEditorUINowIfScheduled();
return hasMarkerFor(DocumentMarker::DictationAlternatives, from, length);
}

return document->frame()->editor().selectionStartHasMarkerFor(DocumentMarker::DictationAlternatives, from, length);
bool Internals::hasCorrectionIndicatorMarker(int from, int length)
{
return hasMarkerFor(DocumentMarker::CorrectionIndicator, from, length);
}

void Internals::setContinuousSpellCheckingEnabled(bool enabled)
Expand Down Expand Up @@ -3018,15 +3021,6 @@ ExceptionOr<void> Internals::setInspectorIsUnderTest(bool isUnderTest)
return { };
}

bool Internals::hasGrammarMarker(int from, int length)
{
Document* document = contextDocument();
if (!document || !document->frame())
return false;

return document->editor().selectionStartHasMarkerFor(DocumentMarker::Grammar, from, length);
}

unsigned Internals::numberOfScrollableAreas()
{
Document* document = contextDocument();
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/testing/Internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "CSSComputedStyleDeclaration.h"
#include "ContextDestructionObserver.h"
#include "Cookie.h"
#include "DocumentMarker.h"
#include "EpochTimeStamp.h"
#include "EventTrackingRegions.h"
#include "ExceptionOr.h"
Expand Down Expand Up @@ -419,6 +420,7 @@ class Internals final : public RefCounted<Internals>, private ContextDestruction
bool hasGrammarMarker(int from, int length);
bool hasAutocorrectedMarker(int from, int length);
bool hasDictationAlternativesMarker(int from, int length);
bool hasCorrectionIndicatorMarker(int from, int length);
void setContinuousSpellCheckingEnabled(bool);
void setAutomaticQuoteSubstitutionEnabled(bool);
void setAutomaticLinkDetectionEnabled(bool);
Expand Down Expand Up @@ -1411,6 +1413,8 @@ class Internals final : public RefCounted<Internals>, private ContextDestruction

CachedResource* resourceFromMemoryCache(const String& url);

bool hasMarkerFor(DocumentMarker::MarkerType, int from, int length);

#if ENABLE(MEDIA_STREAM)
// RealtimeMediaSource::Observer API
void videoFrameAvailable(VideoFrame&, VideoFrameTimeMetadata) final;
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/testing/Internals.idl
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ typedef (FetchRequest or FetchResponse) FetchObject;
boolean hasGrammarMarker(long from, long length);
boolean hasAutocorrectedMarker(long from, long length);
boolean hasDictationAlternativesMarker(long from, long length);
boolean hasCorrectionIndicatorMarker(long from, long length);
undefined setContinuousSpellCheckingEnabled(boolean enabled);
undefined setAutomaticQuoteSubstitutionEnabled(boolean enabled);
undefined setAutomaticLinkDetectionEnabled(boolean enabled);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
- (double)timePickerValueMinute;

- (void)applyAutocorrection:(NSString *)newString toString:(NSString *)oldString withCompletionHandler:(void (^)(void))completionHandler;
- (void)applyAutocorrection:(NSString *)newString toString:(NSString *)oldString isCandidate:(BOOL)isCandidate withCompletionHandler:(void (^)(void))completionHandler;

- (NSDictionary *)_propertiesOfLayerWithID:(unsigned long long)layerID;
- (void)_simulateElementAction:(_WKElementActionType)actionType atLocation:(CGPoint)location;
Expand Down
8 changes: 8 additions & 0 deletions Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ - (void)applyAutocorrection:(NSString *)newString toString:(NSString *)oldString
}];
}

- (void)applyAutocorrection:(NSString *)newString toString:(NSString *)oldString isCandidate:(BOOL)isCandidate withCompletionHandler:(void (^)(void))completionHandler
{
[_contentView applyAutocorrection:newString toString:oldString isCandidate:isCandidate withCompletionHandler:[capturedCompletionHandler = makeBlockPtr(completionHandler)] (UIWKAutocorrectionRects *rects) {
capturedCompletionHandler();
}];
}


- (void)dismissFormAccessoryView
{
[_contentView accessoryDone];
Expand Down
45 changes: 45 additions & 0 deletions Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#import "TestInputDelegate.h"
#import "TestWKWebView.h"
#import "UIKitSPI.h"
#import "WKWebViewConfigurationExtras.h"
#import <WebKit/WKWebViewPrivate.h>
#import <WebKit/WKWebViewPrivateForTesting.h>
#import <WebKit/_WKProcessPoolConfiguration.h>
Expand Down Expand Up @@ -144,6 +145,50 @@ static void checkCGRectIsEqualToCGRectWithLogging(CGRect expected, CGRect observ
EXPECT_EQ(0U, [contextAfterTyping contextAfterSelection].length);
}

#if HAVE(AUTOCORRECTION_ENHANCEMENTS)
TEST(AutocorrectionTests, AutocorrectionIndicatorsDismissAfterNextWord)
{
auto configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];

auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 568) configuration:configuration]);
auto inputDelegate = adoptNS([[TestInputDelegate alloc] init]);
[inputDelegate setFocusStartsInputSessionPolicyHandler:[] (WKWebView *, id<_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
return _WKFocusStartsInputSessionPolicyAllow;
}];

[webView _setInputDelegate:inputDelegate.get()];
[webView synchronouslyLoadTestPageNamed:@"autofocused-text-input"];

auto *contentView = [webView textInputContentView];
[contentView insertText:@"Is it diferent"];

[webView waitForNextPresentationUpdate];

NSString *hasCorrectionIndicatorMarkerJavaScript = @"internals.hasCorrectionIndicatorMarker(6, 9);";

__block bool done = false;

[webView applyAutocorrection:@"different" toString:@"diferent" isCandidate:YES withCompletionHandler:^{
NSString *hasCorrectionIndicatorMarker = [webView stringByEvaluatingJavaScript:hasCorrectionIndicatorMarkerJavaScript];
EXPECT_WK_STREQ("1", hasCorrectionIndicatorMarker);
done = true;
}];

TestWebKitAPI::Util::run(&done);

[contentView insertText:@" than"];
[webView waitForNextPresentationUpdate];

[contentView insertText:@" "];
[webView waitForNextPresentationUpdate];

TestWebKitAPI::Util::runFor(3_s);

NSString *hasCorrectionIndicatorMarker = [webView stringByEvaluatingJavaScript:hasCorrectionIndicatorMarkerJavaScript];
EXPECT_WK_STREQ("0", hasCorrectionIndicatorMarker);
}
#endif

TEST(AutocorrectionTests, AutocorrectionContextBeforeAndAfterEditing)
{
if ([UIKeyboard usesInputSystemUI])
Expand Down

0 comments on commit cafa71f

Please sign in to comment.