Diff
Modified: trunk/LayoutTests/ChangeLog (232661 => 232662)
--- trunk/LayoutTests/ChangeLog 2018-06-09 19:35:33 UTC (rev 232661)
+++ trunk/LayoutTests/ChangeLog 2018-06-09 20:15:28 UTC (rev 232662)
@@ -1,3 +1,17 @@
+2018-06-09 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(macOS Mojave): move-by-word-visually-multi-line.html fails
+ https://bugs.webkit.org/show_bug.cgi?id=186454
+
+ Reviewed by Darin Adler.
+
+ Added a multi-line test case which causes a failure under Mac editing behavior. The test case is symmetric to ml_1.
+
+ * editing/selection/move-by-word-visually-mac-expected.txt:
+ * editing/selection/move-by-word-visually-mac.html:
+ * editing/selection/move-by-word-visually-multi-line-expected.txt:
+ * editing/selection/move-by-word-visually-multi-line.html:
+
2018-06-07 Jer Noble <[email protected]>
REGRESSION: Cannot listen to audio on Google Translate with side switch set to "vibrate"
Modified: trunk/LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt (232661 => 232662)
--- trunk/LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt 2018-06-09 19:35:33 UTC (rev 232661)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt 2018-06-09 20:15:28 UTC (rev 232662)
@@ -92,30 +92,35 @@
"AAA kj AAA mn opq AAA AAA"[25, 22, 18, 14, 11, 7, 4, 0], " abc def AAA AAA hij AAA AAA uvw xyz "[32, 29, 25, 21, 17, 13, 9, 4, 1]
Test 19, LTR:
Move right by one word
+"abc"[0, 3], " def"[4]
+Move left by one word
+" def"[4, 1], "abc"[0]
+Test 20, LTR:
+Move right by one word
"abc def hij opq"[0, 3, 7, 14, 18]
Move left by one word
"abc def hij opq"[18, 15, 8, 4, 0]
-Test 20, LTR:
+Test 21, LTR:
Move right by one word
" abc def hij opq "[4, 7, 14, 21, 28]
Move left by one word
" abc def hij opq "[28, 22, 15, 8, 4]
-Test 21, RTL:
+Test 22, RTL:
Move left by one word
" abc def hij ABW DSU EJH opq rst uvw "[0, 18, 11, 21, 28, 35, 42, 60, 53, 63, 67]
Move right by one word
" abc def hij ABW DSU EJH opq rst uvw "[67, 49, 56, 46, 39, 32, 25, 7, 14, 4, 0]
-Test 22, RTL:
+Test 23, RTL:
Move left by one word
" ABW DSU HJH FUX "[0, 7, 14, 21, 28, 32]
Move right by one word
" ABW DSU HJH FUX "[32, 25, 18, 11, 4, 0]
-Test 23, RTL:
+Test 24, RTL:
Move left by one word
"abc def "[0], " rst uvw"[5, 1], "hij opq"[4], "abc def "[8, 4], " rst uvw"[8]
Move right by one word
" rst uvw"[8], "abc def "[3, 7], "hij opq"[3, 7], " rst uvw"[4], "abc def "[0]
-Test 24, RTL:
+Test 25, RTL:
Move left by one word
"ABD opq rst DSU "[0, 3, 8, 11, 15], "abc uvw AAA def lmn"[16, 12, 11, 4, 19], "ABW hij xyz FXX"[3, 8, 11, 15] FAIL expected: ["ABD opq rst DSU "[ 0, 3, 8, 11, 15, ]"abc uvw AAA def lmn"[ 16, 12, 11, 4, ]"ABW hij xyz FXX"[ 3, 8, 11, 15]
"abc uvw AAA def lmn"[4, 19] FAIL expected "ABW hij xyz FXX"[ 3]
Modified: trunk/LayoutTests/editing/selection/move-by-word-visually-mac.html (232661 => 232662)
--- trunk/LayoutTests/editing/selection/move-by-word-visually-mac.html 2018-06-09 19:35:33 UTC (rev 232661)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-mac.html 2018-06-09 20:15:28 UTC (rev 232662)
@@ -73,6 +73,8 @@
[ml_12, 25, 5][ml_12, 22, 5][ml_12, 18, 5][ml_12, 14, 5][ml_12, 11, 5][ml_12, 7, 5][ml_12, 4, 5][ml_12, 0, 5][ml_12, 32][ml_12, 29][ml_12, 25][ml_12, 21][ml_12, 17][ml_12, 13][ml_12, 9][ml_12, 4][ml_12, 1]|[ml_12, 1][ml_12, 5][ml_12, 8][ml_12, 12][ml_12, 16][ml_12, 20][ml_12, 24][ml_12, 28][ml_12, 33][ml_12, 36][ml_12, 3, 5][ml_12, 6, 5][ml_12, 10, 5][ml_12, 13, 5][ml_12, 17, 5][ml_12, 21, 5][ml_12, 25, 5]
"> abc def אאא אאא hij אאא אאא uvw xyz <div><br/></div><div><br/></div><div><br/></div>אאא kj אאא mn opq אאא אאא</div>
+<div contenteditable dir=ltr id="ml_16" class="test_move_by_word" title="[ml_16, 0][ml_16, 3][ml_16, 4, 4]|[ml_16, 4, 4][ml_16, 1, 4][ml_16, 0]">abc<br><br> def</div>
+
<!-- test multispaces -->
<div dir=ltr class="test_move_by_word" title="0 3 7 14 18|18 15 8 4 0" contenteditable>abc def hij opq</div>
Modified: trunk/LayoutTests/editing/selection/move-by-word-visually-multi-line-expected.txt (232661 => 232662)
--- trunk/LayoutTests/editing/selection/move-by-word-visually-multi-line-expected.txt 2018-06-09 19:35:33 UTC (rev 232661)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-multi-line-expected.txt 2018-06-09 20:15:28 UTC (rev 232662)
@@ -77,15 +77,20 @@
"opq rst uvw xyz"[15, 12, 8, 4, 0], "abc def ghi jkl mn "[16, 12, 8, 4, 0]
Test 16, LTR:
Move right by one word
+"abc"[0], " def"[1, 4]
+Move left by one word
+" def"[4, 1], "abc"[0]
+Test 17, LTR:
+Move right by one word
"abc def "[0, 4, 8]
Move left by one word
" hij opq"[8, 5, 1]
-Test 17, LTR:
+Test 18, LTR:
Move right by one word
<DIV>[0]
Move left by one word
<DIV>[0]
-Test 18, LTR:
+Test 19, LTR:
Move right by one word
"\n00"[3]
Move left by one word
Modified: trunk/LayoutTests/editing/selection/move-by-word-visually-multi-line.html (232661 => 232662)
--- trunk/LayoutTests/editing/selection/move-by-word-visually-multi-line.html 2018-06-09 19:35:33 UTC (rev 232661)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-multi-line.html 2018-06-09 20:15:28 UTC (rev 232662)
@@ -74,6 +74,8 @@
<div contenteditable dir=ltr id="ml_15" class="test_move_by_word fix_width" title="[ml_15, 0][ml_15, 4][ml_15, 8][ml_15, 12][ml_15, 16][ml_15, 0, 5][ml_15, 4, 5][ml_15, 8, 5][ml_15, 12, 5][ml_15, 15, 5]|[ml_15, 15, 5][ml_15, 12, 5][ml_15, 8, 5][ml_15, 4, 5][ml_15, 0, 5][ml_15, 16][ml_15, 12][ml_15, 8][ml_15, 4][ml_15, 0]">abc def ghi jkl mn <div><img src="" rst uvw xyz</div>
+<div contenteditable dir=ltr id="ml_16" class="test_move_by_word" title="[ml_16, 0][ml_16, 1, 4][ml_16, 4, 4]|[ml_16, 4, 4][ml_16, 1, 4][ml_16, 0]">abc<br><br> def</div>
+
<!-- mixed editability -->
<div dir=ltr class="test_move_by_word" title="0 4 8|8 5 1">abc def <span contenteditable> inside span </span> hij opq</div>
Modified: trunk/Source/WebCore/ChangeLog (232661 => 232662)
--- trunk/Source/WebCore/ChangeLog 2018-06-09 19:35:33 UTC (rev 232661)
+++ trunk/Source/WebCore/ChangeLog 2018-06-09 20:15:28 UTC (rev 232662)
@@ -1,3 +1,39 @@
+2018-06-09 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(macOS Mojave): move-by-word-visually-multi-line.html fails
+ https://bugs.webkit.org/show_bug.cgi?id=186454
+
+ Reviewed by Darin Adler.
+
+ Like r232635, this patch fixes a selection test failure caused by the change in ICU's behavior in macOS Mojave,
+ which caused isWordTextBreak to return true in more cases.
+
+ In this particular failing test case, previousTextOrLineBreakBox and nextTextOrLineBreakBox were failing to find
+ an inline text box when it found an inline box for a BR, which was mentioned by an existing FIXME comment.
+ Consequently, visualWordPosition were erroneously detecting the end of a word followed by a blank line created by
+ a BR as a valid word boundary to move when the Windows editing behavior is enacted.
+
+ Addressed the FIXME comment by finding the next inline text box skipping all inline boxes for BRs. Renamed
+ misleadingly named previousBoxInDifferentBlock and nextBoxInDifferentBlock to previousBoxInDifferentLine and
+ nextBoxInDifferentLine respectively, and set them to true as they're really indicating whether line boxes
+ belong to a distinct line or not; whether an inline box belong to two (render) blocks or not is irrelevant.
+
+ Finally, this patch fixes a bug in visualWordPosition that it was failing to skip blank lines when a word break is
+ found as we traversed past a line break. In those cases, we must skip all line breaks before stopping.
+
+ Tests: editing/selection/move-by-word-visually-mac.html
+ editing/selection/move-by-word-visually-multi-line.htm
+
+ * editing/VisibleUnits.cpp:
+ (WebCore::CachedLogicallyOrderedLeafBoxes::previousTextOrLineBreakBox):
+ (WebCore::CachedLogicallyOrderedLeafBoxes::nextTextOrLineBreakBox):
+ (WebCore::CachedLogicallyOrderedLeafBoxes::boxIndexInLeaves const):
+ (WebCore::logicallyPreviousBox):
+ (WebCore::logicallyNextBox):
+ (WebCore::wordBreakIteratorForMinOffsetBoundary):
+ (WebCore::wordBreakIteratorForMaxOffsetBoundary):
+ (WebCore::visualWordPosition):
+
2018-06-09 Zalan Bujtas <[email protected]>
[LFC] MarginCollapse functions should be able to resolve non-fixed margin values
Modified: trunk/Source/WebCore/editing/VisibleUnits.cpp (232661 => 232662)
--- trunk/Source/WebCore/editing/VisibleUnits.cpp 2018-06-09 19:35:33 UTC (rev 232661)
+++ trunk/Source/WebCore/editing/VisibleUnits.cpp 2018-06-09 20:15:28 UTC (rev 232662)
@@ -125,8 +125,8 @@
public:
CachedLogicallyOrderedLeafBoxes();
- const InlineBox* previousTextOrLineBreakBox(const RootInlineBox*, const InlineTextBox*);
- const InlineBox* nextTextOrLineBreakBox(const RootInlineBox*, const InlineTextBox*);
+ const InlineBox* previousTextOrLineBreakBox(const RootInlineBox*, const InlineBox*);
+ const InlineBox* nextTextOrLineBreakBox(const RootInlineBox*, const InlineBox*);
size_t size() const { return m_leafBoxes.size(); }
const InlineBox* firstBox() const { return m_leafBoxes[0]; }
@@ -133,7 +133,7 @@
private:
const Vector<InlineBox*>& collectBoxes(const RootInlineBox*);
- int boxIndexInLeaves(const InlineTextBox*) const;
+ int boxIndexInLeaves(const InlineBox*) const;
const RootInlineBox* m_rootInlineBox { nullptr };
Vector<InlineBox*> m_leafBoxes;
@@ -143,7 +143,7 @@
{
}
-const InlineBox* CachedLogicallyOrderedLeafBoxes::previousTextOrLineBreakBox(const RootInlineBox* root, const InlineTextBox* box)
+const InlineBox* CachedLogicallyOrderedLeafBoxes::previousTextOrLineBreakBox(const RootInlineBox* root, const InlineBox* box)
{
if (!root)
return nullptr;
@@ -164,7 +164,7 @@
return nullptr;
}
-const InlineBox* CachedLogicallyOrderedLeafBoxes::nextTextOrLineBreakBox(const RootInlineBox* root, const InlineTextBox* box)
+const InlineBox* CachedLogicallyOrderedLeafBoxes::nextTextOrLineBreakBox(const RootInlineBox* root, const InlineBox* box)
{
if (!root)
return nullptr;
@@ -196,7 +196,7 @@
return m_leafBoxes;
}
-int CachedLogicallyOrderedLeafBoxes::boxIndexInLeaves(const InlineTextBox* box) const
+int CachedLogicallyOrderedLeafBoxes::boxIndexInLeaves(const InlineBox* box) const
{
for (size_t i = 0; i < m_leafBoxes.size(); ++i) {
if (box == m_leafBoxes[i])
@@ -205,8 +205,8 @@
return 0;
}
-static const InlineBox* logicallyPreviousBox(const VisiblePosition& visiblePosition, const InlineTextBox* textBox,
- bool& previousBoxInDifferentBlock, CachedLogicallyOrderedLeafBoxes& leafBoxes)
+static const InlineBox* logicallyPreviousBox(const VisiblePosition& visiblePosition, const InlineBox* textBox,
+ bool& previousBoxInDifferentLine, CachedLogicallyOrderedLeafBoxes& leafBoxes)
{
const InlineBox* startBox = textBox;
@@ -234,7 +234,7 @@
previousBox = leafBoxes.previousTextOrLineBreakBox(previousRoot, 0);
if (previousBox) {
- previousBoxInDifferentBlock = true;
+ previousBoxInDifferentLine = true;
return previousBox;
}
@@ -246,8 +246,8 @@
}
-static const InlineBox* logicallyNextBox(const VisiblePosition& visiblePosition, const InlineTextBox* textBox,
- bool& nextBoxInDifferentBlock, CachedLogicallyOrderedLeafBoxes& leafBoxes)
+static const InlineBox* logicallyNextBox(const VisiblePosition& visiblePosition, const InlineBox* textBox,
+ bool& nextBoxInDifferentLine, CachedLogicallyOrderedLeafBoxes& leafBoxes)
{
const InlineBox* startBox = textBox;
@@ -275,7 +275,7 @@
nextBox = leafBoxes.nextTextOrLineBreakBox(nextRoot, 0);
if (nextBox) {
- nextBoxInDifferentBlock = true;
+ nextBoxInDifferentLine = true;
return nextBox;
}
@@ -287,12 +287,16 @@
}
static UBreakIterator* wordBreakIteratorForMinOffsetBoundary(const VisiblePosition& visiblePosition, const InlineTextBox* textBox,
- int& previousBoxLength, bool& previousBoxInDifferentBlock, Vector<UChar, 1024>& string, CachedLogicallyOrderedLeafBoxes& leafBoxes)
+ int& previousBoxLength, bool& previousBoxInDifferentLine, Vector<UChar, 1024>& string, CachedLogicallyOrderedLeafBoxes& leafBoxes)
{
- previousBoxInDifferentBlock = false;
+ previousBoxInDifferentLine = false;
- // FIXME: Handle the case when we don't have an inline text box.
- const InlineBox* previousBox = logicallyPreviousBox(visiblePosition, textBox, previousBoxInDifferentBlock, leafBoxes);
+ const InlineBox* previousBox = logicallyPreviousBox(visiblePosition, textBox, previousBoxInDifferentLine, leafBoxes);
+ while (previousBox && !is<InlineTextBox>(previousBox)) {
+ ASSERT(previousBox->renderer().isBR());
+ previousBoxInDifferentLine = true;
+ previousBox = logicallyPreviousBox(visiblePosition, previousBox, previousBoxInDifferentLine, leafBoxes);
+ }
string.clear();
@@ -307,12 +311,16 @@
}
static UBreakIterator* wordBreakIteratorForMaxOffsetBoundary(const VisiblePosition& visiblePosition, const InlineTextBox* textBox,
- bool& nextBoxInDifferentBlock, Vector<UChar, 1024>& string, CachedLogicallyOrderedLeafBoxes& leafBoxes)
+ bool& nextBoxInDifferentLine, Vector<UChar, 1024>& string, CachedLogicallyOrderedLeafBoxes& leafBoxes)
{
- nextBoxInDifferentBlock = false;
+ nextBoxInDifferentLine = false;
- // FIXME: Handle the case when we don't have an inline text box.
- const InlineBox* nextBox = logicallyNextBox(visiblePosition, textBox, nextBoxInDifferentBlock, leafBoxes);
+ const InlineBox* nextBox = logicallyNextBox(visiblePosition, textBox, nextBoxInDifferentLine, leafBoxes);
+ while (nextBox && !is<InlineTextBox>(nextBox)) {
+ ASSERT(nextBox->renderer().isBR());
+ nextBoxInDifferentLine = true;
+ nextBox = logicallyNextBox(visiblePosition, nextBox, nextBoxInDifferentLine, leafBoxes);
+ }
string.clear();
append(string, StringView(textBox->renderer().text()).substring(textBox->start(), textBox->len()));
@@ -379,14 +387,14 @@
InlineTextBox& textBox = downcast<InlineTextBox>(*box);
int previousBoxLength = 0;
- bool previousBoxInDifferentBlock = false;
- bool nextBoxInDifferentBlock = false;
+ bool previousBoxInDifferentLine = false;
+ bool nextBoxInDifferentLine = false;
bool movingIntoNewBox = previouslyVisitedBox != box;
if (offsetInBox == box->caretMinOffset())
- iter = wordBreakIteratorForMinOffsetBoundary(adjacentCharacterPosition, &textBox, previousBoxLength, previousBoxInDifferentBlock, string, leafBoxes);
+ iter = wordBreakIteratorForMinOffsetBoundary(adjacentCharacterPosition, &textBox, previousBoxLength, previousBoxInDifferentLine, string, leafBoxes);
else if (offsetInBox == box->caretMaxOffset())
- iter = wordBreakIteratorForMaxOffsetBoundary(adjacentCharacterPosition, &textBox, nextBoxInDifferentBlock, string, leafBoxes);
+ iter = wordBreakIteratorForMaxOffsetBoundary(adjacentCharacterPosition, &textBox, nextBoxInDifferentLine, string, leafBoxes);
else if (movingIntoNewBox) {
iter = wordBreakIterator(StringView(textBox.renderer().text()).substring(textBox.start(), textBox.len()));
previouslyVisitedBox = box;
@@ -403,11 +411,15 @@
bool movingBackward = (direction == MoveLeft && box->direction() == LTR) || (direction == MoveRight && box->direction() == RTL);
if ((skipsSpaceWhenMovingRight && boxHasSameDirectionalityAsBlock)
|| (!skipsSpaceWhenMovingRight && movingBackward)) {
- bool logicalStartInRenderer = offsetInBox == static_cast<int>(textBox.start()) && previousBoxInDifferentBlock;
+ bool logicalStartInRenderer = offsetInBox == static_cast<int>(textBox.start()) && previousBoxInDifferentLine;
isWordBreak = isLogicalStartOfWord(iter, offsetInIterator, logicalStartInRenderer);
+ if (isWordBreak && offsetInBox == box->caretMaxOffset() && nextBoxInDifferentLine)
+ isWordBreak = false;
} else {
- bool logicalEndInRenderer = offsetInBox == static_cast<int>(textBox.start() + textBox.len()) && nextBoxInDifferentBlock;
+ bool logicalEndInRenderer = offsetInBox == static_cast<int>(textBox.start() + textBox.len()) && nextBoxInDifferentLine;
isWordBreak = islogicalEndOfWord(iter, offsetInIterator, logicalEndInRenderer);
+ if (isWordBreak && offsetInBox == box->caretMinOffset() && previousBoxInDifferentLine)
+ isWordBreak = false;
}
if (isWordBreak)
