Bug 224517 – [iOS] Make image extraction interactions work for elements inside links

• Component: Platform
• Status: RESOLVED FIXED

[Wenson Hsieh]

.

[Radar WebKit Bug Importer]

<rdar://problem/76616431>

[Wenson Hsieh]

Created attachment 425936 [details]
For EWS

[Wenson Hsieh]

Comment on attachment 425936 [details]
For EWS

(Pretty sure the `http/tests/security/webgl-remote-read-remote-image-allowed-with-credentials.html` failure is unrelated)

[Devin Rousso]

Comment on attachment 425936 [details]
For EWS

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

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2698
> +    info.imageElementContext = page.contextForElement(element);

Won't this be the exact same value as `elementContext` since `imagePositionInformation` is called with the same `Element` right before the `elementContext` is set?  If so, would just `isImage` be enough to indicate that the `elementContext` is for an image?

[Wenson Hsieh]

Comment on attachment 425936 [details]
For EWS

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

>> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2698
>> +    info.imageElementContext = page.contextForElement(element);
> 
> Won't this be the exact same value as `elementContext` since `imagePositionInformation` is called with the same `Element` right before the `elementContext` is set?  If so, would just `isImage` be enough to indicate that the `elementContext` is for an image?

They are the same in the `elementPositionInformation` case, but not in the case where the clickable element is a link (<a>).

[Devin Rousso]

Comment on attachment 425936 [details]
For EWS

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

r=me

>>> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2698
>>> +    info.imageElementContext = page.contextForElement(element);
>> 
>> Won't this be the exact same value as `elementContext` since `imagePositionInformation` is called with the same `Element` right before the `elementContext` is set?  If so, would just `isImage` be enough to indicate that the `elementContext` is for an image?
> 
> They are the same in the `elementPositionInformation` case, but not in the case where the clickable element is a link (<a>).

Ah I see are you referring to the `downcast<HTMLImageElement>(*hitTestNode)` below?  That makes sense.

NIT: In that case, are we concerned with having duplicate data in the non-link scenario?  Should we only set the `imageElementContext` when we it's for a different `Element` than the `elementContext`?

[Wenson Hsieh]

Comment on attachment 425936 [details]
For EWS

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

>>>> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2698
>>>> +    info.imageElementContext = page.contextForElement(element);
>>> 
>>> Won't this be the exact same value as `elementContext` since `imagePositionInformation` is called with the same `Element` right before the `elementContext` is set?  If so, would just `isImage` be enough to indicate that the `elementContext` is for an image?
>> 
>> They are the same in the `elementPositionInformation` case, but not in the case where the clickable element is a link (<a>).
> 
> Ah I see are you referring to the `downcast<HTMLImageElement>(*hitTestNode)` below?  That makes sense.
> 
> NIT: In that case, are we concerned with having duplicate data in the non-link scenario?  Should we only set the `imageElementContext` when we it's for a different `Element` than the `elementContext`?

Hm…that could be done, but I feel like that might make the logic that consults this element context more confusing, since it would need to check both the `elementContext` and `imageElementContext` (depending on the `isImage` flag). If it's alright, I think I would prefer to keep `imageElementContext` the way it is in this patch, to make things a bit simpler.

(I'll update the patch to add a test for the link clicking behavior, as we discussed on slack!)

[Wenson Hsieh]

Created attachment 426023 [details]
Add a new test

[Wenson Hsieh]

Created attachment 426024 [details]
Add a new test

[EWS]

Committed r275980 (236532@main): <https://commits.webkit.org/236532@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 426024 [details].