Diff
Modified: branches/safari-605-branch/LayoutTests/ChangeLog (227655 => 227656)
--- branches/safari-605-branch/LayoutTests/ChangeLog 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/LayoutTests/ChangeLog 2018-01-26 05:03:03 UTC (rev 227656)
@@ -1,5 +1,30 @@
2018-01-25 Jason Marcell <[email protected]>
+ Cherry-pick r227533. rdar://problem/36873383
+
+ 2018-01-24 Daniel Bates <[email protected]>
+
+ REGRESSION (r226138): Selecting a line that ends with zero-width joiner (ZWJ) may cause text transformation
+ https://bugs.webkit.org/show_bug.cgi?id=181993
+ <rdar://problem/36421080>
+
+ Reviewed by David Hyatt.
+
+ Add a Mac-specific test to ensure that selecting the last visible character on a line
+ that ends with a zero-width joiner (ZWJ) does not cause a text transformation of the
+ selected character.
+
+ We need to fix <https://bugs.webkit.org/show_bug.cgi?id=181964> for this test to
+ pass on Mac.
+
+ * TestExpectations: Skip the test directory on non-Mac platforms.
+ * fast/text/mac/select-character-before-zero-width-joiner-expected.html: Added.
+ * fast/text/mac/select-character-before-zero-width-joiner.html: Added.
+ * platform/mac/TestExpectations: Mark the test directory as PASS on Mac so that we run
+ all containing tests. Mark the test as ImageOnlyFailure until we fix <https://bugs.webkit.org/show_bug.cgi?id=181964>.
+
+2018-01-25 Jason Marcell <[email protected]>
+
Cherry-pick r227430. rdar://problem/36873610
2018-01-23 Simon Fraser <[email protected]>
Modified: branches/safari-605-branch/LayoutTests/TestExpectations (227655 => 227656)
--- branches/safari-605-branch/LayoutTests/TestExpectations 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/LayoutTests/TestExpectations 2018-01-26 05:03:03 UTC (rev 227656)
@@ -28,6 +28,7 @@
fast/events/touch/ios [ Skip ]
fast/history/ios [ Skip ]
fast/scrolling/ios [ Skip ]
+fast/text/mac [ Skip ]
scrollingcoordinator/ios [ Skip ]
fast/content-observation [ Skip ]
media/mac [ Skip ]
Added: branches/safari-605-branch/LayoutTests/fast/text/mac/select-character-before-zero-width-joiner-expected.html (0 => 227656)
--- branches/safari-605-branch/LayoutTests/fast/text/mac/select-character-before-zero-width-joiner-expected.html (rev 0)
+++ branches/safari-605-branch/LayoutTests/fast/text/mac/select-character-before-zero-width-joiner-expected.html 2018-01-26 05:03:03 UTC (rev 227656)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p id="text" style="font-family: -apple-system">12</p>
+<script>
+var text = document.getElementById("text").firstChild;
+getSelection().setBaseAndExtent(text, 1, text, 2);
+</script>
+</body>
+</html>
Added: branches/safari-605-branch/LayoutTests/fast/text/mac/select-character-before-zero-width-joiner.html (0 => 227656)
--- branches/safari-605-branch/LayoutTests/fast/text/mac/select-character-before-zero-width-joiner.html (rev 0)
+++ branches/safari-605-branch/LayoutTests/fast/text/mac/select-character-before-zero-width-joiner.html 2018-01-26 05:03:03 UTC (rev 227656)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p id="text" style="font-family: -apple-system">12‍</p>
+<script>
+var text = document.getElementById("text").firstChild;
+getSelection().setBaseAndExtent(text, 1, text, 2);
+</script>
+</body>
+</html>
Modified: branches/safari-605-branch/LayoutTests/platform/mac/TestExpectations (227655 => 227656)
--- branches/safari-605-branch/LayoutTests/platform/mac/TestExpectations 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/LayoutTests/platform/mac/TestExpectations 2018-01-26 05:03:03 UTC (rev 227656)
@@ -37,6 +37,9 @@
# FIXME: Mark as Pass once the fix for <rdar://problem/33822249> ships.
http/tests/xmlhttprequest/gzip-content-type-no-content-encoding.html [ Pass Failure ]
+fast/text/mac [ Pass ]
+webkit.org/b/181964 fast/text/mac/select-character-before-zero-width-joiner.html [ ImageOnlyFailure ]
+
#//////////////////////////////////////////////////////////////////////////////////////////
# End platform-specific directories.
#//////////////////////////////////////////////////////////////////////////////////////////
Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (227655 => 227656)
--- branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-26 05:03:03 UTC (rev 227656)
@@ -1,5 +1,66 @@
2018-01-25 Jason Marcell <[email protected]>
+ Cherry-pick r227533. rdar://problem/36873383
+
+ 2018-01-24 Daniel Bates <[email protected]>
+
+ REGRESSION (r226138): Selecting a line that ends with zero-width joiner (ZWJ) may cause text transformation
+ https://bugs.webkit.org/show_bug.cgi?id=181993
+ <rdar://problem/36421080>
+
+ Reviewed by David Hyatt.
+
+ Re-implement paint optimization that was inadvertently removed in r226138. This optimization
+ works around an issue where selecting the last printable character in a line that is followed
+ followed by a zero-width joiner transforms the selected character.
+
+ We need to fix <https://bugs.webkit.org/show_bug.cgi?id=181964> to improve the interaction
+ of selection and zero-width joiner characters. For now, re-implement a paint optimization
+ to perform a single paint operation when the style of the non-selected text is identical
+ to the style of the selected text.
+
+ Test: fast/text/mac/select-character-before-zero-width-joiner.html
+
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::MarkerSubrangeStyle::areBackgroundMarkerSubrangeStylesEqual):
+ (WebCore::InlineTextBox::MarkerSubrangeStyle::areForegroundMarkerSubrangeStylesEqual):
+ (WebCore::InlineTextBox::MarkerSubrangeStyle::areDecorationMarkerSubrangeStylesEqual):
+ Add helper functions to determine when marker styles are identical. We make use of these
+ equality functions to coalesce adjacent subranges that have the same visual style and
+ hence reduce the number of drawing commands to paint all the subranges in a line.
+
+ (WebCore::InlineTextBox::paint): Coalesce subranges before painting.
+
+ (WebCore::InlineTextBox::subdivideAndResolveStyle): Split out the logic to coalesce
+ subranges with the same style into its own function InlineTextBox::coalesceAdjacentSubranges()
+ and kept this function focused on subdivision and style resolution. Manually compute
+ the frontmost subranges so that we can resolve style for each subrange with respect to
+ the correct base style. Formerly we always resolved style with respect the specified
+ base style. Now we resolve style with respect the previous frontmost subrange to ensure
+ styles cascade as expected. This change causes no visual difference now. Once we implement
+ <https://bugs.webkit.org/show_bug.cgi?id=175784> we will be able to test this change
+ with respect to selection of ::spelling-error/::grammar-error pseudo elements.
+
+ (WebCore::InlineTextBox::coalesceAdjacentSubranges): Extracted logic from InlineTextBox::subdivideAndResolveStyle().
+
+ (WebCore::InlineTextBox::MarkerSubrangeStyle::operator== const): Deleted.
+ (WebCore::InlineTextBox::MarkerSubrangeStyle::operator!= const): Deleted.
+ Comparing MarkerSubrangeStyle objects should be performed using the appropriate
+ are*MarkerSubrangeStylesEqual() non-member function.
+
+ * rendering/InlineTextBox.h:
+ * rendering/MarkerSubrange.cpp:
+ (WebCore::subdivide): Remove overlap strategy FrontmostWithLongestEffectiveRange
+ as this strategy is now implemented by InlineTextBox::subdivideAndResolveStyle() and
+ InlineTextBox::coalesceAdjacentSubranges() that compute the set of frontmost subranges and
+ coalesces adjacent subranges with the same style into the longest effective subrange,
+ respectively. Unlike WebCore::subdivide(), InlineTextBox knows what the base style should
+ be for the subranges and can more aggressively coalesce adjacent subranges of different
+ types that have the same visual style.
+ * rendering/MarkerSubrange.h:
+
+2018-01-25 Jason Marcell <[email protected]>
+
Cherry-pick r227430. rdar://problem/36873610
2018-01-23 Simon Fraser <[email protected]>
Modified: branches/safari-605-branch/Source/WebCore/rendering/InlineTextBox.cpp (227655 => 227656)
--- branches/safari-605-branch/Source/WebCore/rendering/InlineTextBox.cpp 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/Source/WebCore/rendering/InlineTextBox.cpp 2018-01-26 05:03:03 UTC (rev 227656)
@@ -393,13 +393,20 @@
}
struct InlineTextBox::MarkerSubrangeStyle {
- bool operator==(const MarkerSubrangeStyle& other) const
+ bool operator==(const MarkerSubrangeStyle& other) const = delete;
+ bool operator!=(const MarkerSubrangeStyle& other) const = delete;
+ static bool areBackgroundMarkerSubrangeStylesEqual(const MarkerSubrangeStyle& a, const MarkerSubrangeStyle& b)
{
- return backgroundColor == other.backgroundColor && textStyles == other.textStyles
- && textDecorationStyles == other.textDecorationStyles && textShadow == other.textShadow
- && alpha == other.alpha;
+ return a.backgroundColor == b.backgroundColor;
}
- bool operator!=(const MarkerSubrangeStyle& other) const { return !(*this == other); }
+ static bool areForegroundMarkerSubrangeStylesEqual(const MarkerSubrangeStyle& a, const MarkerSubrangeStyle& b)
+ {
+ return a.textStyles == b.textStyles && a.textShadow == b.textShadow && a.alpha == b.alpha;
+ }
+ static bool areDecorationMarkerSubrangeStylesEqual(const MarkerSubrangeStyle& a, const MarkerSubrangeStyle& b)
+ {
+ return a.textDecorationStyles == b.textDecorationStyles && a.textShadow == b.textShadow;
+ }
Color backgroundColor;
TextPaintStyle textStyles;
@@ -510,7 +517,11 @@
}
#endif
auto styledSubranges = subdivideAndResolveStyle(subranges, unmarkedStyle, paintInfo);
- paintMarkerSubranges(context, TextPaintPhase::Background, boxRect, styledSubranges);
+
+ // Coalesce styles of adjacent subranges to minimize the number of drawing commands.
+ auto coalescedStyledSubranges = coalesceAdjacentSubranges(styledSubranges, &MarkerSubrangeStyle::areBackgroundMarkerSubrangeStylesEqual);
+
+ paintMarkerSubranges(context, TextPaintPhase::Background, boxRect, coalescedStyledSubranges);
}
// FIXME: Right now, InlineTextBoxes never call addRelevantUnpaintedObject() even though they might
@@ -553,8 +564,11 @@
if (!isPrinting && paintInfo.paintBehavior & PaintBehaviorExcludeSelection)
styledSubranges.removeAllMatching([] (const StyledMarkerSubrange& subrange) { return subrange.type == MarkerSubrange::Selection; });
- paintMarkerSubranges(context, TextPaintPhase::Foreground, boxRect, styledSubranges);
+ // Coalesce styles of adjacent subranges to minimize the number of drawing commands.
+ auto coalescedStyledSubranges = coalesceAdjacentSubranges(styledSubranges, &MarkerSubrangeStyle::areForegroundMarkerSubrangeStylesEqual);
+ paintMarkerSubranges(context, TextPaintPhase::Foreground, boxRect, coalescedStyledSubranges);
+
// Paint decorations
TextDecoration textDecorations = lineStyle.textDecorationsInEffect();
if (textDecorations != TextDecorationNone && paintInfo.phase != PaintPhaseSelection) {
@@ -587,7 +601,11 @@
textDecorationSelectionClipOutRect.setWidth(logicalSelectionWidth);
}
}
- paintMarkerSubranges(context, TextPaintPhase::Decoration, boxRect, styledSubranges, textDecorationSelectionClipOutRect);
+
+ // Coalesce styles of adjacent subranges to minimize the number of drawing commands.
+ auto coalescedStyledSubranges = coalesceAdjacentSubranges(styledSubranges, &MarkerSubrangeStyle::areDecorationMarkerSubrangeStylesEqual);
+
+ paintMarkerSubranges(context, TextPaintPhase::Decoration, boxRect, coalescedStyledSubranges, textDecorationSelectionClipOutRect);
}
// 3. Paint fancy decorations, including composition underlines and platform-specific underlines for spelling errors, grammar errors, et cetera.
@@ -765,25 +783,46 @@
if (subrangesToSubdivide.isEmpty())
return { };
- auto areAdjacentSubrangesWithSameStyle = [] (const StyledMarkerSubrange& a, const StyledMarkerSubrange& b) {
- return a.endOffset == b.startOffset && a.style == b.style;
+ auto subranges = subdivide(subrangesToSubdivide);
+
+ // Compute frontmost overlapping styled subranges.
+ Vector<StyledMarkerSubrange> frontmostSubranges;
+ frontmostSubranges.reserveInitialCapacity(subranges.size());
+ frontmostSubranges.uncheckedAppend(resolveStyleForSubrange(subranges[0], baseStyle, paintInfo));
+ for (auto it = subranges.begin() + 1, end = subranges.end(); it != end; ++it) {
+ StyledMarkerSubrange& previousStyledSubrange = frontmostSubranges.last();
+ if (previousStyledSubrange.startOffset == it->startOffset && previousStyledSubrange.endOffset == it->endOffset) {
+ // Subranges completely cover each other.
+ previousStyledSubrange = resolveStyleForSubrange(*it, previousStyledSubrange.style, paintInfo);
+ continue;
+ }
+ frontmostSubranges.uncheckedAppend(resolveStyleForSubrange(*it, baseStyle, paintInfo));
+ }
+
+ return frontmostSubranges;
+}
+
+auto InlineTextBox::coalesceAdjacentSubranges(const Vector<StyledMarkerSubrange>& subrangesToCoalesce, MarkerSubrangeStylesEqualityFunction areMarkerSubrangeStylesEqual) -> Vector<StyledMarkerSubrange>
+{
+ if (subrangesToCoalesce.isEmpty())
+ return { };
+
+ auto areAdjacentSubrangesWithSameStyle = [&] (const StyledMarkerSubrange& a, const StyledMarkerSubrange& b) {
+ return a.endOffset == b.startOffset && areMarkerSubrangeStylesEqual(a.style, b.style);
};
- auto subranges = subdivide(subrangesToSubdivide, OverlapStrategy::FrontmostWithLongestEffectiveRange);
-
- // Coallesce styles of adjacent subranges to minimize the number of drawing commands.
Vector<StyledMarkerSubrange> styledSubranges;
- styledSubranges.reserveInitialCapacity(subranges.size());
- styledSubranges.uncheckedAppend(resolveStyleForSubrange(subranges[0], baseStyle, paintInfo));
- for (auto it = subranges.begin() + 1, end = subranges.end(); it != end; ++it) {
+ styledSubranges.reserveInitialCapacity(subrangesToCoalesce.size());
+ styledSubranges.uncheckedAppend(subrangesToCoalesce[0]);
+ for (auto it = subrangesToCoalesce.begin() + 1, end = subrangesToCoalesce.end(); it != end; ++it) {
StyledMarkerSubrange& previousStyledSubrange = styledSubranges.last();
- auto currentStyledSubrange = resolveStyleForSubrange(*it, baseStyle, paintInfo);
- if (areAdjacentSubrangesWithSameStyle(previousStyledSubrange, currentStyledSubrange)) {
- previousStyledSubrange.endOffset = currentStyledSubrange.endOffset;
+ if (areAdjacentSubrangesWithSameStyle(previousStyledSubrange, *it)) {
+ previousStyledSubrange.endOffset = it->endOffset;
continue;
}
- styledSubranges.uncheckedAppend(currentStyledSubrange);
+ styledSubranges.uncheckedAppend(*it);
}
+
return styledSubranges;
}
Modified: branches/safari-605-branch/Source/WebCore/rendering/InlineTextBox.h (227655 => 227656)
--- branches/safari-605-branch/Source/WebCore/rendering/InlineTextBox.h 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/Source/WebCore/rendering/InlineTextBox.h 2018-01-26 05:03:03 UTC (rev 227656)
@@ -163,6 +163,9 @@
StyledMarkerSubrange resolveStyleForSubrange(const MarkerSubrange&, const MarkerSubrangeStyle& baseStyle, const PaintInfo&);
Vector<StyledMarkerSubrange> subdivideAndResolveStyle(const Vector<MarkerSubrange>&, const MarkerSubrangeStyle& baseStyle, const PaintInfo&);
+ using MarkerSubrangeStylesEqualityFunction = bool (*)(const MarkerSubrangeStyle&, const MarkerSubrangeStyle&);
+ Vector<StyledMarkerSubrange> coalesceAdjacentSubranges(const Vector<StyledMarkerSubrange>&, MarkerSubrangeStylesEqualityFunction);
+
FloatPoint textOriginFromBoxRect(const FloatRect&) const;
void paintMarkerSubranges(GraphicsContext&, TextPaintPhase, const FloatRect& boxRect, const Vector<StyledMarkerSubrange>&, const FloatRect& decorationClipOutRect = { });
Modified: branches/safari-605-branch/Source/WebCore/rendering/MarkerSubrange.cpp (227655 => 227656)
--- branches/safari-605-branch/Source/WebCore/rendering/MarkerSubrange.cpp 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/Source/WebCore/rendering/MarkerSubrange.cpp 2018-01-26 05:03:03 UTC (rev 227656)
@@ -67,22 +67,14 @@
unsigned offsetSoFar = offsets[0].value;
for (unsigned i = 1; i < numberOfOffsets; ++i) {
if (offsets[i].value > offsets[i - 1].value) {
- if (overlapStrategy == OverlapStrategy::Frontmost || overlapStrategy == OverlapStrategy::FrontmostWithLongestEffectiveRange) {
+ if (overlapStrategy == OverlapStrategy::Frontmost) {
std::optional<unsigned> frontmost;
for (unsigned j = 0; j < i; ++j) {
if (!processedSubranges.contains(offsets[j].subrange) && (!frontmost || offsets[j].subrange->type > offsets[*frontmost].subrange->type))
frontmost = j;
}
- if (frontmost) {
- if (overlapStrategy == OverlapStrategy::FrontmostWithLongestEffectiveRange && !result.isEmpty()) {
- auto& previous = result.last();
- if (previous.endOffset == offsetSoFar && previous.type == offsets[*frontmost].subrange->type && previous.marker == offsets[*frontmost].subrange->marker)
- previous.endOffset = offsets[i].value;
- else
- result.append({ offsetSoFar, offsets[i].value, offsets[*frontmost].subrange->type, offsets[*frontmost].subrange->marker });
- } else
- result.append({ offsetSoFar, offsets[i].value, offsets[*frontmost].subrange->type, offsets[*frontmost].subrange->marker });
- }
+ if (frontmost)
+ result.append({ offsetSoFar, offsets[i].value, offsets[*frontmost].subrange->type, offsets[*frontmost].subrange->marker });
} else {
// The appended subranges may not be in paint order. We will fix this up at the end of this function.
for (unsigned j = 0; j < i; ++j) {
Modified: branches/safari-605-branch/Source/WebCore/rendering/MarkerSubrange.h (227655 => 227656)
--- branches/safari-605-branch/Source/WebCore/rendering/MarkerSubrange.h 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/Source/WebCore/rendering/MarkerSubrange.h 2018-01-26 05:03:03 UTC (rev 227656)
@@ -70,7 +70,7 @@
}
};
-enum class OverlapStrategy { None, Frontmost, FrontmostWithLongestEffectiveRange };
+enum class OverlapStrategy { None, Frontmost };
WEBCORE_EXPORT Vector<MarkerSubrange> subdivide(const Vector<MarkerSubrange>&, OverlapStrategy = OverlapStrategy::None);
}
Modified: branches/safari-605-branch/Tools/ChangeLog (227655 => 227656)
--- branches/safari-605-branch/Tools/ChangeLog 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/Tools/ChangeLog 2018-01-26 05:03:03 UTC (rev 227656)
@@ -1,5 +1,22 @@
2018-01-25 Jason Marcell <[email protected]>
+ Cherry-pick r227533. rdar://problem/36873383
+
+ 2018-01-24 Daniel Bates <[email protected]>
+
+ REGRESSION (r226138): Selecting a line that ends with zero-width joiner (ZWJ) may cause text transformation
+ https://bugs.webkit.org/show_bug.cgi?id=181993
+ <rdar://problem/36421080>
+
+ Reviewed by David Hyatt.
+
+ Remove unit test for overlap strategy FrontmostWithLongestEffectiveRange
+ as we no longer support this strategy.
+
+ * TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp:
+
+2018-01-25 Jason Marcell <[email protected]>
+
Cherry-pick r227430. rdar://problem/36873610
2018-01-23 Simon Fraser <[email protected]>
Modified: branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp (227655 => 227656)
--- branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp 2018-01-26 05:02:59 UTC (rev 227655)
+++ branches/safari-605-branch/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp 2018-01-26 05:03:03 UTC (rev 227656)
@@ -214,21 +214,4 @@
EXPECT_EQ(expectedSubranges[i], results[i]);
}
-TEST(MarkerSubrange, SubdivideGrammarAndSelectionOverlapFrontmostWithLongestEffectiveRange)
-{
- Vector<MarkerSubrange> subranges {
- MarkerSubrange { 0, 40, MarkerSubrange::GrammarError },
- MarkerSubrange { 2, 60, MarkerSubrange::Selection },
- MarkerSubrange { 50, 60, MarkerSubrange::GrammarError },
- };
- Vector<MarkerSubrange> expectedSubranges {
- MarkerSubrange { 0, 2, MarkerSubrange::GrammarError },
- MarkerSubrange { 2, 60, MarkerSubrange::Selection },
- };
- auto results = subdivide(subranges, OverlapStrategy::FrontmostWithLongestEffectiveRange);
- ASSERT_EQ(expectedSubranges.size(), results.size());
- for (size_t i = 0; i < expectedSubranges.size(); ++i)
- EXPECT_EQ(expectedSubranges[i], results[i]);
}
-
-}
