- Revision
- 278775
- Author
- wenson_hs...@apple.com
- Date
- 2021-06-11 12:55:13 -0700 (Fri, 11 Jun 2021)
Log Message
[Live Text] Text selection inside image elements should not be cleared upon resize
https://bugs.webkit.org/show_bug.cgi?id=226911
Reviewed by Tim Horton.
Source/WebCore:
Refactor `HTMLElement::updateWithTextRecognitionResult`, such that it doesn't tear down and recreate the host
element's shadow DOM structure in the case where the extant DOM elements are compatible with the given text
recognition result. This prevents us from removing or inserting DOM elements in the case where an image element
is resized (and thus adjusts its shadow DOM content using the updated size), which in turn prevents us from
clearing out the text selection.
Test: fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change.html
* editing/cocoa/DataDetection.h:
* editing/cocoa/DataDetection.mm:
Make this helper method return an HTMLDivElement instead of just an HTMLElement.
(WebCore::DataDetection::createElementForImageOverlay):
* html/HTMLElement.cpp:
(WebCore::HTMLElement::updateWithTextRecognitionResult):
Split this method into two logical parts: the first builds up a TextRecognitionElements struct that contains
references to all connected elements in the image element's shadow DOM that require style updates due to the
new size; the second uses this TextRecognitionElements information to compute the new CSS transforms to apply to
each of the data detector, line containers, and text containers underneath each line container element.
Importantly, in step (1), we avoid regenerating shadow DOM content in the case where the DOM elements already
exist in their expected places within the shadow DOM.
LayoutTests:
* fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change-expected.txt: Added.
* fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (278774 => 278775)
--- trunk/LayoutTests/ChangeLog 2021-06-11 19:14:34 UTC (rev 278774)
+++ trunk/LayoutTests/ChangeLog 2021-06-11 19:55:13 UTC (rev 278775)
@@ -1,3 +1,13 @@
+2021-06-11 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [Live Text] Text selection inside image elements should not be cleared upon resize
+ https://bugs.webkit.org/show_bug.cgi?id=226911
+
+ Reviewed by Tim Horton.
+
+ * fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change-expected.txt: Added.
+ * fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change.html: Added.
+
2021-06-11 Cathie Chen <cathiec...@igalia.com>
Use HTMLDimension to parse different HTML attribute length values
Added: trunk/LayoutTests/fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change-expected.txt (0 => 278775)
--- trunk/LayoutTests/fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change-expected.txt 2021-06-11 19:55:13 UTC (rev 278775)
@@ -0,0 +1,10 @@
+PASS selectionWidth is 200
+PASS selectionHeight is 200
+PASS selectionWidth became 50
+PASS selectionHeight is 50
+PASS selectionWidth became 200
+PASS selectionHeight is 200
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change.html (0 => 278775)
--- trunk/LayoutTests/fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change.html (rev 0)
+++ trunk/LayoutTests/fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change.html 2021-06-11 19:55:13 UTC (rev 278775)
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html>
+<script src=""
+<body>
+<img src=""
+<script>
+jsTestIsAsync = true;
+let image = document.querySelector("img");
+let selectionWidth = 0;
+let selectionHeight = 0;
+
+function updateSelectionDimensions() {
+ let selectionBounds = internals.selectionBounds();
+ selectionWidth = selectionBounds.width;
+ selectionHeight = selectionBounds.height;
+}
+
+addEventListener("load", async () => {
+ for (let i = 0; i < 10; ++i) {
+ internals.installImageOverlay(image, [
+ {
+ topLeft : new DOMPointReadOnly(0.5, 0.5),
+ topRight : new DOMPointReadOnly(1, 0.5),
+ bottomRight : new DOMPointReadOnly(1, 1),
+ bottomLeft : new DOMPointReadOnly(0.5, 1),
+ children: [
+ {
+ text : "Hello",
+ topLeft : new DOMPointReadOnly(0.5, 0.5),
+ topRight : new DOMPointReadOnly(1, 0.5),
+ bottomRight : new DOMPointReadOnly(1, 1),
+ bottomLeft : new DOMPointReadOnly(0.5, 1),
+ }
+ ],
+ }
+ ]);
+ }
+
+ getSelection().selectAllChildren(internals.shadowRoot(image).querySelector(".image-overlay-text"));
+ updateSelectionDimensions();
+
+ shouldBe("selectionWidth", "200");
+ shouldBe("selectionHeight", "200");
+
+ image.style.width = "100px";
+ setInterval(updateSelectionDimensions, 10);
+
+ await shouldBecomeEqual("selectionWidth", "50");
+ shouldBe("selectionHeight", "50");
+
+ image.style.width = "";
+ await shouldBecomeEqual("selectionWidth", "200");
+ shouldBe("selectionHeight", "200");
+
+ finishJSTest();
+});
+</script>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (278774 => 278775)
--- trunk/Source/WebCore/ChangeLog 2021-06-11 19:14:34 UTC (rev 278774)
+++ trunk/Source/WebCore/ChangeLog 2021-06-11 19:55:13 UTC (rev 278775)
@@ -1,3 +1,35 @@
+2021-06-11 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [Live Text] Text selection inside image elements should not be cleared upon resize
+ https://bugs.webkit.org/show_bug.cgi?id=226911
+
+ Reviewed by Tim Horton.
+
+ Refactor `HTMLElement::updateWithTextRecognitionResult`, such that it doesn't tear down and recreate the host
+ element's shadow DOM structure in the case where the extant DOM elements are compatible with the given text
+ recognition result. This prevents us from removing or inserting DOM elements in the case where an image element
+ is resized (and thus adjusts its shadow DOM content using the updated size), which in turn prevents us from
+ clearing out the text selection.
+
+ Test: fast/images/text-recognition/mac/image-overlay-maintain-selection-during-size-change.html
+
+ * editing/cocoa/DataDetection.h:
+ * editing/cocoa/DataDetection.mm:
+
+ Make this helper method return an HTMLDivElement instead of just an HTMLElement.
+
+ (WebCore::DataDetection::createElementForImageOverlay):
+ * html/HTMLElement.cpp:
+ (WebCore::HTMLElement::updateWithTextRecognitionResult):
+
+ Split this method into two logical parts: the first builds up a TextRecognitionElements struct that contains
+ references to all connected elements in the image element's shadow DOM that require style updates due to the
+ new size; the second uses this TextRecognitionElements information to compute the new CSS transforms to apply to
+ each of the data detector, line containers, and text containers underneath each line container element.
+
+ Importantly, in step (1), we avoid regenerating shadow DOM content in the case where the DOM elements already
+ exist in their expected places within the shadow DOM.
+
2021-06-11 Megan Gardner <megan_gard...@apple.com>
Rename AppHighlight group to QuickNote to correctly reflect feature.
Modified: trunk/Source/WebCore/editing/cocoa/DataDetection.h (278774 => 278775)
--- trunk/Source/WebCore/editing/cocoa/DataDetection.h 2021-06-11 19:14:34 UTC (rev 278774)
+++ trunk/Source/WebCore/editing/cocoa/DataDetection.h 2021-06-11 19:55:13 UTC (rev 278775)
@@ -40,7 +40,7 @@
namespace WebCore {
class Document;
-class HTMLElement;
+class HTMLDivElement;
class HitTestResult;
class QualifiedName;
struct TextRecognitionDataDetector;
@@ -67,7 +67,7 @@
#endif
#if ENABLE(IMAGE_ANALYSIS)
- static Ref<HTMLElement> createElementForImageOverlay(Document&, const TextRecognitionDataDetector&);
+ static Ref<HTMLDivElement> createElementForImageOverlay(Document&, const TextRecognitionDataDetector&);
#endif
static const String& dataDetectorURLProtocol();
Modified: trunk/Source/WebCore/editing/cocoa/DataDetection.mm (278774 => 278775)
--- trunk/Source/WebCore/editing/cocoa/DataDetection.mm 2021-06-11 19:14:34 UTC (rev 278774)
+++ trunk/Source/WebCore/editing/cocoa/DataDetection.mm 2021-06-11 19:55:13 UTC (rev 278775)
@@ -689,7 +689,7 @@
#if ENABLE(IMAGE_ANALYSIS)
-Ref<HTMLElement> DataDetection::createElementForImageOverlay(Document& document, const TextRecognitionDataDetector& info)
+Ref<HTMLDivElement> DataDetection::createElementForImageOverlay(Document& document, const TextRecognitionDataDetector& info)
{
auto container = HTMLDivElement::create(document);
if (auto frame = makeRefPtr(document.frame())) {
Modified: trunk/Source/WebCore/html/HTMLElement.cpp (278774 => 278775)
--- trunk/Source/WebCore/html/HTMLElement.cpp 2021-06-11 19:14:34 UTC (rev 278774)
+++ trunk/Source/WebCore/html/HTMLElement.cpp 2021-06-11 19:55:13 UTC (rev 278775)
@@ -1341,23 +1341,121 @@
void HTMLElement::updateWithTextRecognitionResult(const TextRecognitionResult& result, CacheTextRecognitionResults cacheTextRecognitionResults)
{
- RefPtr<HTMLDivElement> previousContainer;
- if (auto shadowRoot = userAgentShadowRoot(); shadowRoot && hasImageOverlay()) {
- for (auto& child : childrenOfType<HTMLDivElement>(*shadowRoot)) {
+ static MainThreadNeverDestroyed<const AtomString> imageOverlayLineClass("image-overlay-line", AtomString::ConstructFromLiteral);
+ static MainThreadNeverDestroyed<const AtomString> imageOverlayTextClass("image-overlay-text", AtomString::ConstructFromLiteral);
+
+ struct TextRecognitionLineElements {
+ Ref<HTMLDivElement> line;
+ Vector<Ref<HTMLDivElement>> children;
+ };
+
+ struct TextRecognitionElements {
+ RefPtr<HTMLDivElement> root;
+ Vector<TextRecognitionLineElements> lines;
+ Vector<Ref<HTMLDivElement>> dataDetectors;
+ };
+
+ bool hadExistingTextRecognitionElements = false;
+ TextRecognitionElements textRecognitionElements;
+
+ if (hasImageOverlay()) {
+ for (auto& child : childrenOfType<HTMLDivElement>(*userAgentShadowRoot())) {
if (child.getIdAttribute() == imageOverlayElementIdentifier()) {
- previousContainer = &child;
+ textRecognitionElements.root = &child;
+ hadExistingTextRecognitionElements = true;
break;
}
}
- if (previousContainer)
- previousContainer->remove();
- else
- ASSERT_NOT_REACHED();
}
+ if (textRecognitionElements.root) {
+ for (auto& lineOrDataDetector : childrenOfType<HTMLDivElement>(*textRecognitionElements.root)) {
+ if (!lineOrDataDetector.hasClass())
+ continue;
+
+ if (lineOrDataDetector.classList().contains(imageOverlayLineClass)) {
+ TextRecognitionLineElements lineElements { lineOrDataDetector };
+ for (auto& text : childrenOfType<HTMLDivElement>(lineOrDataDetector))
+ lineElements.children.append(text);
+ textRecognitionElements.lines.append(WTFMove(lineElements));
+ } else if (lineOrDataDetector.classList().contains(imageOverlayDataDetectorClassName()))
+ textRecognitionElements.dataDetectors.append(lineOrDataDetector);
+ }
+
+ bool canUseExistingTextRecognitionElements = ([&] {
+ if (result.dataDetectors.size() != textRecognitionElements.dataDetectors.size())
+ return false;
+
+ if (result.lines.size() != textRecognitionElements.lines.size())
+ return false;
+
+ for (size_t lineIndex = 0; lineIndex < result.lines.size(); ++lineIndex) {
+ auto& childResults = result.lines[lineIndex].children;
+ auto& childTextElements = textRecognitionElements.lines[lineIndex].children;
+ if (childResults.size() != childTextElements.size())
+ return false;
+
+ for (size_t childIndex = 0; childIndex < childResults.size(); ++childIndex) {
+ if (childResults[childIndex].text != childTextElements[childIndex]->textContent().stripWhiteSpace())
+ return false;
+ }
+ }
+
+ return true;
+ })();
+
+ if (!canUseExistingTextRecognitionElements) {
+ textRecognitionElements.root->remove();
+ textRecognitionElements = { };
+ }
+ }
+
if (result.isEmpty())
return;
+ auto shadowRoot = makeRef(ensureUserAgentShadowRoot());
+ bool hasUserSelectNone = renderer() && renderer()->style().userSelect() == UserSelect::None;
+ if (!textRecognitionElements.root) {
+ auto rootContainer = HTMLDivElement::create(document());
+ rootContainer->setIdAttribute(imageOverlayElementIdentifier());
+ if (document().isImageDocument())
+ rootContainer->setInlineStyleProperty(CSSPropertyWebkitUserSelect, CSSValueText);
+ shadowRoot->appendChild(rootContainer);
+ textRecognitionElements.root = rootContainer.copyRef();
+ textRecognitionElements.lines.reserveInitialCapacity(result.lines.size());
+ for (auto& line : result.lines) {
+ auto lineContainer = HTMLDivElement::create(document());
+ lineContainer->classList().add(imageOverlayLineClass);
+ rootContainer->appendChild(lineContainer);
+ TextRecognitionLineElements lineElements { lineContainer };
+ lineElements.children.reserveInitialCapacity(line.children.size());
+ for (size_t childIndex = 0; childIndex < line.children.size(); ++childIndex) {
+ auto& child = line.children[childIndex];
+ auto textContainer = HTMLDivElement::create(document());
+ textContainer->classList().add(imageOverlayTextClass);
+ lineContainer->appendChild(textContainer);
+ textContainer->appendChild(Text::create(document(), makeString('\n', child.text)));
+ lineElements.children.uncheckedAppend(WTFMove(textContainer));
+ }
+
+ lineContainer->appendChild(HTMLBRElement::create(document()));
+ textRecognitionElements.lines.uncheckedAppend(WTFMove(lineElements));
+ }
+
+#if ENABLE(DATA_DETECTION)
+ textRecognitionElements.dataDetectors.reserveInitialCapacity(result.dataDetectors.size());
+ for (auto& dataDetector : result.dataDetectors) {
+ auto dataDetectorContainer = DataDetection::createElementForImageOverlay(document(), dataDetector);
+ dataDetectorContainer->classList().add(imageOverlayDataDetectorClassName());
+ rootContainer->appendChild(dataDetectorContainer);
+ textRecognitionElements.dataDetectors.uncheckedAppend(WTFMove(dataDetectorContainer));
+ }
+#endif // ENABLE(DATA_DETECTION)
+
+ if (document().quirks().needsToForceUserSelectWhenInstallingImageOverlay())
+ setInlineStyleProperty(CSSPropertyWebkitUserSelect, CSSValueText);
+ }
+
if (auto* renderer = this->renderer()) {
if (!is<RenderImage>(renderer))
return;
@@ -1365,8 +1463,7 @@
downcast<RenderImage>(*renderer).setHasImageOverlay();
}
- auto shadowRoot = makeRef(ensureUserAgentShadowRoot());
- if (!previousContainer) {
+ if (!hadExistingTextRecognitionElements) {
static MainThreadNeverDestroyed<const String> shadowStyle(StringImpl::createWithoutCopying(imageOverlayUserAgentStyleSheet, sizeof(imageOverlayUserAgentStyleSheet)));
auto style = HTMLStyleElement::create(HTMLNames::styleTag, document(), false);
style->setTextContent(shadowStyle);
@@ -1373,21 +1470,11 @@
shadowRoot->appendChild(WTFMove(style));
}
- auto container = HTMLDivElement::create(document());
- container->setIdAttribute(imageOverlayElementIdentifier());
- if (document().isImageDocument())
- container->setInlineStyleProperty(CSSPropertyWebkitUserSelect, CSSValueText);
- shadowRoot->appendChild(container);
-
- if (document().quirks().needsToForceUserSelectWhenInstallingImageOverlay())
- setInlineStyleProperty(CSSPropertyWebkitUserSelect, CSSValueText);
-
- static MainThreadNeverDestroyed<const AtomString> imageOverlayLineClass("image-overlay-line", AtomString::ConstructFromLiteral);
- static MainThreadNeverDestroyed<const AtomString> imageOverlayTextClass("image-overlay-text", AtomString::ConstructFromLiteral);
-
- bool hasUserSelectNone = renderer() && renderer()->style().userSelect() == UserSelect::None;
IntSize containerSize { offsetWidth(), offsetHeight() };
- for (auto& line : result.lines) {
+ for (size_t lineIndex = 0; lineIndex < result.lines.size(); ++lineIndex) {
+ auto& lineElements = textRecognitionElements.lines[lineIndex];
+ auto& lineContainer = lineElements.line;
+ auto& line = result.lines[lineIndex];
auto lineQuad = line.normalizedQuad;
if (lineQuad.isEmpty())
continue;
@@ -1394,10 +1481,6 @@
lineQuad.scale(containerSize.width(), containerSize.height());
- auto lineContainer = HTMLDivElement::create(document());
- lineContainer->classList().add(imageOverlayLineClass);
- container->appendChild(lineContainer);
-
auto lineBounds = rotatedBoundingRectWithMinimumAngleOfRotation(lineQuad, 0.01);
lineContainer->setInlineStyleProperty(CSSPropertyWidth, lineBounds.size.width(), CSSUnitType::CSS_PX);
lineContainer->setInlineStyleProperty(CSSPropertyHeight, lineBounds.size.height(), CSSUnitType::CSS_PX);
@@ -1426,10 +1509,7 @@
});
for (size_t childIndex = 0; childIndex < line.children.size(); ++childIndex) {
- auto& child = line.children[childIndex];
- if (child.normalizedQuad.isEmpty())
- continue;
-
+ auto& textContainer = lineElements.children[childIndex];
bool lineHasOneChild = line.children.size() == 1;
float horizontalMarginToMinimizeSelectionGaps = lineHasOneChild ? 0 : 0.125;
float horizontalOffset = lineHasOneChild ? 0 : -horizontalMarginToMinimizeSelectionGaps;
@@ -1450,14 +1530,11 @@
}
FloatSize targetSize { horizontalExtent - horizontalOffset, lineBounds.size.height() };
- if (targetSize.isEmpty())
+ if (targetSize.isEmpty()) {
+ textContainer->setInlineStyleProperty(CSSPropertyTransform, "scale(0, 0)");
continue;
+ }
- auto textContainer = HTMLDivElement::create(document());
- textContainer->classList().add(imageOverlayTextClass);
- lineContainer->appendChild(textContainer);
- textContainer->appendChild(Text::create(document(), makeString('\n', child.text)));
-
document().updateLayoutIfDimensionsOutOfDate(textContainer);
FloatSize sizeBeforeTransform;
@@ -1469,7 +1546,7 @@
}
if (sizeBeforeTransform.isEmpty()) {
- textContainer->remove();
+ textContainer->setInlineStyleProperty(CSSPropertyTransform, "scale(0, 0)");
continue;
}
@@ -1491,14 +1568,12 @@
}
#if ENABLE(DATA_DETECTION)
- for (auto& dataDetector : result.dataDetectors) {
+ for (size_t index = 0; index < result.dataDetectors.size(); ++index) {
+ auto dataDetectorContainer = textRecognitionElements.dataDetectors[index];
+ auto& dataDetector = result.dataDetectors[index];
if (dataDetector.normalizedQuads.isEmpty())
continue;
- auto dataDetectorContainer = DataDetection::createElementForImageOverlay(document(), dataDetector);
- dataDetectorContainer->classList().add(imageOverlayDataDetectorClassName());
- container->appendChild(dataDetectorContainer);
-
// FIXME: We should come up with a way to coalesce the bounding quads into one or more rotated rects with the same angle of rotation.
auto targetQuad = dataDetector.normalizedQuads.first();
targetQuad.scale(containerSize.width(), containerSize.height());
