Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Live Text] Avoid injecting recognized text consisting of single words or letters in tiny images #4512

Merged
merged 1 commit into from Sep 20, 2022

Conversation

whsieh
Copy link
Member

@whsieh whsieh commented Sep 20, 2022

03e0d55

[Live Text] Avoid injecting recognized text consisting of single words or letters in tiny images
https://bugs.webkit.org/show_bug.cgi?id=245404
rdar://99490753

Reviewed by Tim Horton.

Add a basic heuristic to avoid injecting Live Text in small images, if the recognized text consists
of only a single word or letter. Currently, this causes images to become selectable in many
instances where there's little to no utility in being able to select text — for instance, in image
buttons or links that consist of an icon, such as an "x".

Test: fast/images/text-recognition/avoid-image-overlay-in-small-image.html

* LayoutTests/fast/images/text-recognition/avoid-image-overlay-in-small-image-expected.txt: Added.
* LayoutTests/fast/images/text-recognition/avoid-image-overlay-in-small-image.html: Added.
* Source/WebCore/dom/ImageOverlay.cpp:
(WebCore::ImageOverlay::updateWithTextRecognitionResult):

To do this, we shuffle around some logic for creating and appending the UA shadow DOM elements used
to implement Live Text, such that we exit early and avoid installing a shadow root altogether in the
case where the image is smaller than an arbitrary threshold (64px), and the text we're about to
inject consists of a single piece of child text.

Canonical link: https://commits.webkit.org/254671@main

e9a99f8

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🛠 mac-AS-debug ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 api-mac ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk1
✅ 🛠 tv-sim ✅ 🧪 mac-wk2
✅ 🛠 🧪 merge ✅ 🛠 watch 🧪 mac-AS-debug-wk2
✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress

@whsieh whsieh self-assigned this Sep 20, 2022
@whsieh whsieh added Platform Portability improvements and other general platform improvements not driven directly by site bugs. WebKit Nightly Build labels Sep 20, 2022
if (!elements.root)
return;

{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this separate scope needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I wrote it this way to ensure that nothing below tries to reuse renderer here. If anything updates layout below and causes the image to recreate its renderer, then this pointer could point to a destroyed object.

@whsieh whsieh added the merge-queue Applied to send a pull request to merge-queue label Sep 20, 2022
@whsieh
Copy link
Member Author

whsieh commented Sep 20, 2022

Thanks for the reviews!

…s or letters in tiny images

https://bugs.webkit.org/show_bug.cgi?id=245404
rdar://99490753

Reviewed by Tim Horton.

Add a basic heuristic to avoid injecting Live Text in small images, if the recognized text consists
of only a single word or letter. Currently, this causes images to become selectable in many
instances where there's little to no utility in being able to select text — for instance, in image
buttons or links that consist of an icon, such as an "x".

Test: fast/images/text-recognition/avoid-image-overlay-in-small-image.html

* LayoutTests/fast/images/text-recognition/avoid-image-overlay-in-small-image-expected.txt: Added.
* LayoutTests/fast/images/text-recognition/avoid-image-overlay-in-small-image.html: Added.
* Source/WebCore/dom/ImageOverlay.cpp:
(WebCore::ImageOverlay::updateWithTextRecognitionResult):

To do this, we shuffle around some logic for creating and appending the UA shadow DOM elements used
to implement Live Text, such that we exit early and avoid installing a shadow root altogether in the
case where the image is smaller than an arbitrary threshold (64px), and the text we're about to
inject consists of a single piece of child text.

Canonical link: https://commits.webkit.org/254671@main
@webkit-commit-queue
Copy link
Collaborator

Committed 254671@main (03e0d55): https://commits.webkit.org/254671@main

Reviewed commits have been landed. Closing PR #4512 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 03e0d55 into WebKit:main Sep 20, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform Portability improvements and other general platform improvements not driven directly by site bugs.
Projects
None yet
5 participants