<rdar://problem/73203775>
Created attachment 422391 [details] Patch
Created attachment 422392 [details] Patch
Created attachment 422402 [details] Fix non-internal iOS build
Thanks for the review! (Looks like I need a slight adjustment to fix the !ENABLE(IMAGE_EXTRACTION) build)
Comment on attachment 422402 [details] Fix non-internal iOS build View in context: https://bugs.webkit.org/attachment.cgi?id=422402&action=review r=me as well :) > Source/WebKit/ChangeLog:27 > + There's no need to make the regular text interaction duck in favor of the image extraction interaction after "duck"? > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2737 > + if (HTMLElement::isImageOverlayText(*innerNonSharedNode)) Can/Should we assume that this is not `nullptr`? Should we have `HTMLElement::isImageOverlayText` take a `const Node*` instead?
(In reply to Devin Rousso from comment #5) > Comment on attachment 422402 [details] > Fix non-internal iOS build > > View in context: > https://bugs.webkit.org/attachment.cgi?id=422402&action=review > > r=me as well :) > > > Source/WebKit/ChangeLog:27 > > + There's no need to make the regular text interaction duck in favor of the image extraction interaction after > > "duck"? As in, "make one thing duck to avoid another thing", but perhaps that isn't so clear — I'll reword this sentence. > > > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2737 > > + if (HTMLElement::isImageOverlayText(*innerNonSharedNode)) > > Can/Should we assume that this is not `nullptr`? Should we have > `HTMLElement::isImageOverlayText` take a `const Node*` instead? Since this is a hit test result, I'm a bit hesitant to assume that it must be non-null.
Created attachment 422407 [details] Patch
Comment on attachment 422407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422407&action=review > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:9412 > + block(NO); Use an enum? > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:9944 > + getConfigurationAndContinue(NO); Use an enum?
(In reply to Simon Fraser (smfr) from comment #8) > Comment on attachment 422407 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=422407&action=review > > > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:9412 > > + block(NO); > > Use an enum? Good call — added this enum, and changed the block to take it instead of BOOL: enum class ProceedWithImageExtraction : bool { No, Yes }; > > > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:9944 > > + getConfigurationAndContinue(NO); > > Use an enum?
Created attachment 422428 [details] For EWS
Created attachment 422444 [details] Fix non-internal iOS build
Created attachment 422445 [details] Fix non-internal iOS build
Created attachment 422450 [details] Patch
Committed r274031: <https://commits.webkit.org/r274031> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422450 [details].