- Revision
- 287866
- Author
- [email protected]
- Date
- 2022-01-10 18:17:09 -0800 (Mon, 10 Jan 2022)
Log Message
Followup to r287863 - adjust line wrapping behavior in image overlays
https://bugs.webkit.org/show_bug.cgi?id=235035
rdar://85139146
Reviewed by Tim Horton.
Source/WebCore:
The previous change I landed in r287863 was written under the assumption that `-shouldWrap` describes whether or
not a recognized line of text should wrap to the next line. However, after trying it out, it seems to indicate
whether or not the previous line of text should wrap to the current line. As a result, the current adoption of
this property is wrong, since the line wrapping is off-by-one.
Fix this by renaming the `shouldWrap` boolean flag in TextRecognitionLineData to the much less ambiguous name
`hasTrailingNewline`, and set `hasTrailingNewline` based on the `shouldWrap` property of the *next*
VKWKLineInfo.
* dom/ImageOverlay.cpp:
(WebCore::ImageOverlay::updateSubtree):
Avoid a potentially even-more-confusing `!= !!` here by using `static_cast<>` to check the nullity of the
`RefPtr`.
* platform/TextRecognitionResult.h:
(WebCore::TextRecognitionLineData::TextRecognitionLineData):
(WebCore::TextRecognitionLineData::encode const):
(WebCore::TextRecognitionLineData::decode):
Make sure we also flip the default from `false` to `true`, since `hasTrailingNewline` is intended to have the
opposite effect as the extant `shouldWrap`.
* testing/Internals.cpp:
(WebCore::makeDataForLine):
* testing/Internals.h:
* testing/Internals.idl:
Source/WebKit:
Use the `-shouldWrap` property to compute `hasTrailingNewline`. See WebCore/ChangeLog for more details.
* Platform/cocoa/TextRecognitionUtilities.mm:
(WebKit::makeTextRecognitionResult):
LayoutTests:
Make a slight adjustment to an existing layout test (i.e. rename `shouldWrap` to `hasTrailingNewline`). See
WebCore/ChangeLog for more details.
* fast/images/text-recognition/image-overlay-line-wrapping.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (287865 => 287866)
--- trunk/LayoutTests/ChangeLog 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/LayoutTests/ChangeLog 2022-01-11 02:17:09 UTC (rev 287866)
@@ -1,5 +1,18 @@
2022-01-10 Wenson Hsieh <[email protected]>
+ Followup to r287863 - adjust line wrapping behavior in image overlays
+ https://bugs.webkit.org/show_bug.cgi?id=235035
+ rdar://85139146
+
+ Reviewed by Tim Horton.
+
+ Make a slight adjustment to an existing layout test (i.e. rename `shouldWrap` to `hasTrailingNewline`). See
+ WebCore/ChangeLog for more details.
+
+ * fast/images/text-recognition/image-overlay-line-wrapping.html:
+
+2022-01-10 Wenson Hsieh <[email protected]>
+
Add support for a -shouldWrap property when injecting Live Text
https://bugs.webkit.org/show_bug.cgi?id=235035
rdar://85139146
Modified: trunk/LayoutTests/fast/images/text-recognition/image-overlay-line-wrapping.html (287865 => 287866)
--- trunk/LayoutTests/fast/images/text-recognition/image-overlay-line-wrapping.html 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/LayoutTests/fast/images/text-recognition/image-overlay-line-wrapping.html 2022-01-11 02:17:09 UTC (rev 287866)
@@ -25,7 +25,7 @@
topRight : new DOMPointReadOnly(1, 0),
bottomRight : new DOMPointReadOnly(1, 0.3),
bottomLeft : new DOMPointReadOnly(0, 0.3),
- shouldWrap: true,
+ hasTrailingNewline: false,
children: [
{
text : "this",
Modified: trunk/Source/WebCore/ChangeLog (287865 => 287866)
--- trunk/Source/WebCore/ChangeLog 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/Source/WebCore/ChangeLog 2022-01-11 02:17:09 UTC (rev 287866)
@@ -1,5 +1,41 @@
2022-01-10 Wenson Hsieh <[email protected]>
+ Followup to r287863 - adjust line wrapping behavior in image overlays
+ https://bugs.webkit.org/show_bug.cgi?id=235035
+ rdar://85139146
+
+ Reviewed by Tim Horton.
+
+ The previous change I landed in r287863 was written under the assumption that `-shouldWrap` describes whether or
+ not a recognized line of text should wrap to the next line. However, after trying it out, it seems to indicate
+ whether or not the previous line of text should wrap to the current line. As a result, the current adoption of
+ this property is wrong, since the line wrapping is off-by-one.
+
+ Fix this by renaming the `shouldWrap` boolean flag in TextRecognitionLineData to the much less ambiguous name
+ `hasTrailingNewline`, and set `hasTrailingNewline` based on the `shouldWrap` property of the *next*
+ VKWKLineInfo.
+
+ * dom/ImageOverlay.cpp:
+ (WebCore::ImageOverlay::updateSubtree):
+
+ Avoid a potentially even-more-confusing `!= !!` here by using `static_cast<>` to check the nullity of the
+ `RefPtr`.
+
+ * platform/TextRecognitionResult.h:
+ (WebCore::TextRecognitionLineData::TextRecognitionLineData):
+ (WebCore::TextRecognitionLineData::encode const):
+ (WebCore::TextRecognitionLineData::decode):
+
+ Make sure we also flip the default from `false` to `true`, since `hasTrailingNewline` is intended to have the
+ opposite effect as the extant `shouldWrap`.
+
+ * testing/Internals.cpp:
+ (WebCore::makeDataForLine):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+2022-01-10 Wenson Hsieh <[email protected]>
+
Add support for a -shouldWrap property when injecting Live Text
https://bugs.webkit.org/show_bug.cgi?id=235035
rdar://85139146
Modified: trunk/Source/WebCore/dom/ImageOverlay.cpp (287865 => 287866)
--- trunk/Source/WebCore/dom/ImageOverlay.cpp 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/Source/WebCore/dom/ImageOverlay.cpp 2022-01-11 02:17:09 UTC (rev 287866)
@@ -276,7 +276,7 @@
auto& lineElements = elements.lines[lineIndex];
auto& childTextElements = lineElements.children;
- if (lineResult.shouldWrap != !lineElements.lineBreak)
+ if (lineResult.hasTrailingNewline != static_cast<bool>(lineElements.lineBreak))
return false;
if (childResults.size() != childTextElements.size())
@@ -335,7 +335,7 @@
lineElements.children.uncheckedAppend(WTFMove(textContainer));
}
- if (!line.shouldWrap) {
+ if (line.hasTrailingNewline) {
lineElements.lineBreak = HTMLBRElement::create(document.get());
lineContainer->appendChild(*lineElements.lineBreak);
}
Modified: trunk/Source/WebCore/platform/TextRecognitionResult.h (287865 => 287866)
--- trunk/Source/WebCore/platform/TextRecognitionResult.h 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/Source/WebCore/platform/TextRecognitionResult.h 2022-01-11 02:17:09 UTC (rev 287866)
@@ -81,16 +81,16 @@
}
struct TextRecognitionLineData {
- TextRecognitionLineData(FloatQuad&& quad, Vector<TextRecognitionWordData>&& theChildren, bool wrap)
+ TextRecognitionLineData(FloatQuad&& quad, Vector<TextRecognitionWordData>&& theChildren, bool newline)
: normalizedQuad(WTFMove(quad))
, children(WTFMove(theChildren))
- , shouldWrap(wrap)
+ , hasTrailingNewline(newline)
{
}
FloatQuad normalizedQuad;
Vector<TextRecognitionWordData> children;
- bool shouldWrap { false };
+ bool hasTrailingNewline { true };
template<class Encoder> void encode(Encoder&) const;
template<class Decoder> static std::optional<TextRecognitionLineData> decode(Decoder&);
@@ -116,7 +116,7 @@
{
encoder << normalizedQuad;
encoder << children;
- encoder << shouldWrap;
+ encoder << hasTrailingNewline;
}
template<class Decoder> std::optional<TextRecognitionLineData> TextRecognitionLineData::decode(Decoder& decoder)
@@ -131,12 +131,12 @@
if (!children)
return std::nullopt;
- std::optional<bool> shouldWrap;
- decoder >> shouldWrap;
- if (!shouldWrap)
+ std::optional<bool> hasTrailingNewline;
+ decoder >> hasTrailingNewline;
+ if (!hasTrailingNewline)
return std::nullopt;
- return { { WTFMove(*normalizedQuad), WTFMove(*children), *shouldWrap } };
+ return { { WTFMove(*normalizedQuad), WTFMove(*children), *hasTrailingNewline } };
}
struct TextRecognitionBlockData {
Modified: trunk/Source/WebCore/testing/Internals.cpp (287865 => 287866)
--- trunk/Source/WebCore/testing/Internals.cpp 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/Source/WebCore/testing/Internals.cpp 2022-01-11 02:17:09 UTC (rev 287866)
@@ -5770,7 +5770,7 @@
line.children.map([](auto& textChild) -> TextRecognitionWordData {
return { textChild.text, getQuad<Internals::ImageOverlayText>(textChild), textChild.hasLeadingWhitespace };
}),
- line.shouldWrap
+ line.hasTrailingNewline
};
}
Modified: trunk/Source/WebCore/testing/Internals.h (287865 => 287866)
--- trunk/Source/WebCore/testing/Internals.h 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/Source/WebCore/testing/Internals.h 2022-01-11 02:17:09 UTC (rev 287866)
@@ -922,7 +922,7 @@
RefPtr<DOMPointReadOnly> bottomRight;
RefPtr<DOMPointReadOnly> bottomLeft;
Vector<ImageOverlayText> children;
- bool shouldWrap { false };
+ bool hasTrailingNewline { true };
~ImageOverlayLine();
};
Modified: trunk/Source/WebCore/testing/Internals.idl (287865 => 287866)
--- trunk/Source/WebCore/testing/Internals.idl 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/Source/WebCore/testing/Internals.idl 2022-01-11 02:17:09 UTC (rev 287866)
@@ -292,7 +292,7 @@
required DOMPointReadOnly bottomRight;
required DOMPointReadOnly bottomLeft;
sequence<ImageOverlayText> children;
- boolean shouldWrap = false;
+ boolean hasTrailingNewline = true;
};
[
Modified: trunk/Source/WebKit/ChangeLog (287865 => 287866)
--- trunk/Source/WebKit/ChangeLog 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/Source/WebKit/ChangeLog 2022-01-11 02:17:09 UTC (rev 287866)
@@ -1,5 +1,18 @@
2022-01-10 Wenson Hsieh <[email protected]>
+ Followup to r287863 - adjust line wrapping behavior in image overlays
+ https://bugs.webkit.org/show_bug.cgi?id=235035
+ rdar://85139146
+
+ Reviewed by Tim Horton.
+
+ Use the `-shouldWrap` property to compute `hasTrailingNewline`. See WebCore/ChangeLog for more details.
+
+ * Platform/cocoa/TextRecognitionUtilities.mm:
+ (WebKit::makeTextRecognitionResult):
+
+2022-01-10 Wenson Hsieh <[email protected]>
+
Add support for a -shouldWrap property when injecting Live Text
https://bugs.webkit.org/show_bug.cgi?id=235035
rdar://85139146
Modified: trunk/Source/WebKit/Platform/cocoa/TextRecognitionUtilities.mm (287865 => 287866)
--- trunk/Source/WebKit/Platform/cocoa/TextRecognitionUtilities.mm 2022-01-11 02:01:49 UTC (rev 287865)
+++ trunk/Source/WebKit/Platform/cocoa/TextRecognitionUtilities.mm 2022-01-11 02:17:09 UTC (rev 287866)
@@ -78,11 +78,12 @@
TextRecognitionResult makeTextRecognitionResult(VKImageAnalysis *analysis)
{
- NSArray<VKWKTextInfo *> *allLines = analysis.allLines;
+ NSArray<VKWKLineInfo *> *allLines = analysis.allLines;
TextRecognitionResult result;
result.lines.reserveInitialCapacity(allLines.count);
bool isFirstLine = true;
+ size_t nextLineIndex = 1;
for (VKWKLineInfo *line in allLines) {
Vector<TextRecognitionWordData> children;
NSArray<VKWKTextInfo *> *vkChildren = line.children;
@@ -114,12 +115,12 @@
searchLocation = matchLocation + childText.length();
children.uncheckedAppend({ WTFMove(childText), floatQuad(child.quad), hasLeadingWhitespace });
}
- result.lines.uncheckedAppend({
- floatQuad(line.quad),
- WTFMove(children),
- [line respondsToSelector:@selector(shouldWrap)] && [line shouldWrap]
- });
+ VKWKLineInfo *nextLine = nextLineIndex < allLines.count ? allLines[nextLineIndex] : nil;
+ // The `shouldWrap` property indicates whether or not a line should wrap, relative to the previous line.
+ bool hasTrailingNewline = nextLine && (![nextLine respondsToSelector:@selector(shouldWrap)] || ![nextLine shouldWrap]);
+ result.lines.uncheckedAppend({ floatQuad(line.quad), WTFMove(children), hasTrailingNewline });
isFirstLine = false;
+ nextLineIndex++;
}
#if ENABLE(DATA_DETECTION)
