Return annotated text checking strings via UIWKDocumentContext
Created attachment 367971 [details] Patch
<rdar://problem/49064839>
Comment on attachment 367971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367971&action=review > Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.h:54 > + AttributedString annotatedSubstringBetweenPositions(WebCore::VisiblePosition, WebCore::VisiblePosition); Use const &? > Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:171 > + RefPtr<Document> document = start.deepEquivalent().document(); makeRefPtr? > Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:185 > + RefPtr<Range> currentTextRange = it.range(); It's expensive to create range. Do this after the continue below. > Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:190 > + [string replaceCharactersInRange:NSMakeRange(stringLength, 0) withString:it.text().createNSStringWithoutCopying().get()]; Why not just appendString? > Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:202 > + auto subrange = TextIterator::subrange(*currentTextRange, marker->startOffset(), marker->endOffset() - marker->startOffset()); > + > + size_t subrangeLocation; > + size_t subrangeLength; > + TextIterator::getLocationAndLengthFromRange(commonAncestor.get(), &subrange.get(), subrangeLocation, subrangeLength); This is a pretty bad o(n^2)...
Comment on attachment 367971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367971&action=review >> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:171 >> + RefPtr<Document> document = start.deepEquivalent().document(); > > makeRefPtr? What's the benefit?
(In reply to Ryosuke Niwa from comment #3) > Comment on attachment 367971 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=367971&action=review > > > Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:202 > > + auto subrange = TextIterator::subrange(*currentTextRange, marker->startOffset(), marker->endOffset() - marker->startOffset()); > > + > > + size_t subrangeLocation; > > + size_t subrangeLength; > > + TextIterator::getLocationAndLengthFromRange(commonAncestor.get(), &subrange.get(), subrangeLocation, subrangeLength); > > This is a pretty bad o(n^2)... Yeah this whole thing is awful. Is there a straightforward better way, though?
Created attachment 368079 [details] Patch
Comment on attachment 368079 [details] Patch Clearing flags on attachment: 368079 Committed r244570: <https://trac.webkit.org/changeset/244570>
All reviewed patches have been landed. Closing bug.
(In reply to Chris Dumez from comment #4) > Comment on attachment 367971 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=367971&action=review > > >> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:171 > >> + RefPtr<Document> document = start.deepEquivalent().document(); > > > > makeRefPtr? > > What's the benefit? The benefit is that we can use auto as in: auto document = makeRefPtr(start.deepEquivalent().document());
(In reply to Ryosuke Niwa from comment #9) > (In reply to Chris Dumez from comment #4) > > Comment on attachment 367971 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=367971&action=review > > > > >> Source/WebKit/WebProcess/WebPage/Cocoa/TextCheckingControllerProxy.mm:171 > > >> + RefPtr<Document> document = start.deepEquivalent().document(); > > > > > > makeRefPtr? > > > > What's the benefit? > > The benefit is that we can use auto as in: > auto document = makeRefPtr(start.deepEquivalent().document()); ah, with "auto". I get it now :)