.
Created attachment 453349 [details] Patch
Created attachment 453350 [details] Fix watchOS build
Comment on attachment 453350 [details] Fix watchOS build View in context: https://bugs.webkit.org/attachment.cgi?id=453350&action=review > Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:494 > + RefPtr<Element> selectionHost = originalSelection.rootEditableElement() ?: document->body(); Should be able to just write RefPtr here, not RefPtr<Element>. > Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:518 > + auto newSelectionRange = resolveCharacterRange(makeRangeSelectingNodeContents(*selectionHost), *rangeToRestore, iteratorOptions); Seems like this will do the wrong thing if the replacement doesn’t have the same number of characters as the selection beforehand. We may need some other form of selection preservation to handle interesting cases well.
Comment on attachment 453350 [details] Fix watchOS build View in context: https://bugs.webkit.org/attachment.cgi?id=453350&action=review Thanks for the review! >> Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:494 >> + RefPtr<Element> selectionHost = originalSelection.rootEditableElement() ?: document->body(); > > Should be able to just write RefPtr here, not RefPtr<Element>. Fixed! >> Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:518 >> + auto newSelectionRange = resolveCharacterRange(makeRangeSelectingNodeContents(*selectionHost), *rangeToRestore, iteratorOptions); > > Seems like this will do the wrong thing if the replacement doesn’t have the same number of characters as the selection beforehand. We may need some other form of selection preservation to handle interesting cases well. Ah, that's a good point! For the time being, this should only ever be invoked in circumstances where we're swapping one image element with another (which should preserve the character count), but if this codepath is ever used to replace a node with text, this restoration logic won't work at all. I'll add an early return for now to avoid trying to restore the selection if the number of visible characters underneath the editable root has changed, with a FIXME for smarter handling around this in the future, should we need it.
Created attachment 453356 [details] Patch
Committed r290578 (247856@main): <https://commits.webkit.org/247856@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453356 [details].
<rdar://problem/89537723>