Bug 208406 – Batch observations and completions of text manipulations

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

We should patch findings of paragraphs and completions of text manipulations.

<rdar://problem/59643650>

[Ryosuke Niwa]

Created attachment 392051 [details]
WIP

[Ryosuke Niwa]

Created attachment 392250 [details]
Patch

[Ryosuke Niwa]

Created attachment 392323 [details]
Fixed build

[Wenson Hsieh]

Comment on attachment 392323 [details]
Fixed build

View in context: https://bugs.webkit.org/attachment.cgi?id=392323&action=review

> Source/WebCore/editing/TextManipulationController.cpp:279
> +        m_callback(*m_document, m_pendingItemsForCallback);
> +        m_pendingItemsForCallback.clear();

Nit - you could call flushPendingItemsForCallback() here instead of repeating the code.

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1658
> +        // WebCore::TextManipulationController::ItemIdentifier itemID, const Vector<WebCore::TextManipulationController::ManipulationToken>& tokens

Nit - stray comment?

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1694
> +            [wkItems addObject:[[_WKTextManipulationItem alloc] initWithIdentifier:String::number(item.identifier.toUInt64()) tokens:wkTokens]];

This appears to be a leak — use RetainPtr/adoptNS or -autorelease?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:75
> +        for (_WKTextManipulationItem* item in items)

Nit - * on the other side.

[Ryosuke Niwa]

(In reply to Wenson Hsieh from comment #4)
> Comment on attachment 392323 [details]
> Fixed build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=392323&action=review
> 
> > Source/WebCore/editing/TextManipulationController.cpp:279
> > +        m_callback(*m_document, m_pendingItemsForCallback);
> > +        m_pendingItemsForCallback.clear();
> 
> Nit - you could call flushPendingItemsForCallback() here instead of
> repeating the code.

Good idea. Done.

> > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1658
> > +        // WebCore::TextManipulationController::ItemIdentifier itemID, const Vector<WebCore::TextManipulationController::ManipulationToken>& tokens
> 
> Nit - stray comment?

Oops, removed.

> > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1694
> > +            [wkItems addObject:[[_WKTextManipulationItem alloc] initWithIdentifier:String::number(item.identifier.toUInt64()) tokens:wkTokens]];
> 
> This appears to be a leak — use RetainPtr/adoptNS or -autorelease?

Oh oops, fixed it by adoptNS into a local variable first.

> > Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:75
> > +        for (_WKTextManipulationItem* item in items)
> 
> Nit - * on the other side.

Fixed.

[Ryosuke Niwa]

Committed r257830: <https://trac.webkit.org/changeset/257830>