This can muck with whitespace addition/removal with iOS smart editing.
<rdar://problem/64779558>
Created attachment 404306 [details] Patch
Created attachment 404311 [details] Patch
Created attachment 404312 [details] Patch ... stupid semicolons (╯°□°)╯︵ ┻━┻
Comment on attachment 404311 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404311&action=review > Source/WebCore/editing/TextIterator.cpp:807 > +static bool shouldEmitReplacementInsteadOfNode(Node& node) This should probably take `const Node&` as the argument type. > Source/WebCore/editing/TextIterator.cpp:851 > + // Placeholders should eventually disappear, so treating them as a line break doesn't make sense > + // as when they are removed the text after it is combined with the text before it. Nit - I think this comment would be more effective inside `shouldEmitReplacementInsteadOfNode`, since that is where the notion of “emitting replacement characters instead of nodes" becomes relevant to `TextPlaceholderElement`. > Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h:61 > +- (void)_insertTextPlaceholderWithSize:(CGSize)size completionHandler:(void (^)(UITextPlaceholder *))completionHandler; > +- (void)_removeTextPlaceholder:(UITextPlaceholder *)placeholder willInsertText:(BOOL)willInsertText completionHandler:(void (^)(void))completionHandler; Nit - I think you could actually omit this extra plumbing on WKWebView, and just use -textInputContentView instead to grab the WKContentView (exposed as a <UIWKInteractionViewProtocol>-conformant responder). > Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm:117 > +- (void)_insertTextPlaceholderWithSize:(CGSize)size completionHandler:(void (^)(UITextPlaceholder *))completionHandler > +{ > + [_contentView insertTextPlaceholderWithSize:size completionHandler:completionHandler]; > +} > + > +- (void)_removeTextPlaceholder:(UITextPlaceholder *)placeholder willInsertText:(BOOL)willInsertText completionHandler:(void (^)(void))completionHandler > +{ > + [_contentView removeTextPlaceholder:placeholder willInsertText:willInsertText completionHandler:completionHandler]; > +} > + (Ditto) > Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:863 > + auto* placeholder = [webView synchronouslyInsertTextPlaceholderWithSize:CGSizeMake(5, 5)]; Nit - the * should be on the other side here. But also, we can just use `auto` for these. > Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:880 > + auto* placeholder = [webView synchronouslyInsertTextPlaceholderWithSize:CGSizeMake(5, 5)]; Ditto.
Comment on attachment 404312 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404312&action=review > Source/WebKit/Platform/spi/ios/UIKitSPI.h:708 > +- (void)insertTextPlaceholderWithSize:(CGSize)size completionHandler:(void (^)(UITextPlaceholder *))completionHandler; > +- (void)removeTextPlaceholder:(UITextPlaceholder *)placeholder willInsertText:(BOOL)willInsertText completionHandler:(void (^)(void))completionHandler; This should also go inside the `@optional` section, to match the actual SPI header.
Comment on attachment 404311 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404311&action=review >> Source/WebCore/editing/TextIterator.cpp:807 >> +static bool shouldEmitReplacementInsteadOfNode(Node& node) > > This should probably take `const Node&` as the argument type. I was matching the other `shouldEmit*` functions, but yeah I could do that.
Created attachment 404314 [details] Patch
Created attachment 404355 [details] Patch
Comment on attachment 404355 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404355&action=review > Source/WebCore/ChangeLog:9 > + Reviewed by NOBODY (OOPS!). > + > + * editing/TextIterator.cpp: It would be nice to have a few words here to explain why we want to emit the replacement character instead of a newline (something similar to what you already wrote in the comment below).
Created attachment 404376 [details] Patch
Committed r264420: <https://trac.webkit.org/changeset/264420> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404376 [details].
Comment on attachment 404376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404376&action=review > Source/WebCore/editing/TextIterator.cpp:1014 > + emitCharacter(0xFFFC, *m_node->parentNode(), m_node, 0, 0); Should write: objectReplacementCharacter Instead of: 0xFFFC It’s in the header CharacterNames.h.
Comment on attachment 404376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404376&action=review >> Source/WebCore/editing/TextIterator.cpp:1014 >> + emitCharacter(0xFFFC, *m_node->parentNode(), m_node, 0, 0); > > Should write: > > objectReplacementCharacter > > Instead of: > > 0xFFFC > > It’s in the header CharacterNames.h. Oh neat! I did not know that existed :) Will land a followup.
Committed r264439: Unreviewed followup, use objectReplacementCharacter instead of 0xFFFC