- Revision
- 185393
- Author
- [email protected]
- Date
- 2015-06-09 16:49:49 -0700 (Tue, 09 Jun 2015)
Log Message
Takes two delete key presses to delete pasted emoji up-pointing index finger with skin tone
https://bugs.webkit.org/show_bug.cgi?id=145823
Reviewed by Anders Carlsson.
Source/WebCore:
Tests: editing/deleting/delete-emoji.html
* rendering/RenderText.cpp:
(WebCore::isHangulLVT): Use constants instead of macros. Also changed to take a UChar since
the Hangul processing can work on UTF-16 code unit at a time and doesn't have to handle
surrogate pairs.
(WebCore::isMark): Use U_GC_M_MASK instead of writing the algorithm out another way.
(WebCore::isInArmenianToLimbuRange): Added.
(WebCore::RenderText::previousOffsetForBackwardDeletion): Refactored for clarity and to use
the U16_PREV macro instead of doing what it does in a sloppier way. Added code to allow a
variation selector before an emoji modifier to fix the bug. Changed Hangul logic to work a
code unit at a time, since it can, to use an enum class, and to use constants rather than
all capital macros. Also changed the "dumb" case to use a more appropriate ICU macro.
LayoutTests:
* editing/deleting/delete-emoji-expected.txt: Updated to expect a little more testing.
* editing/deleting/delete-emoji.html: Added a test case and streamlined the test a bit.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (185392 => 185393)
--- trunk/LayoutTests/ChangeLog 2015-06-09 23:24:12 UTC (rev 185392)
+++ trunk/LayoutTests/ChangeLog 2015-06-09 23:49:49 UTC (rev 185393)
@@ -1,3 +1,13 @@
+2015-06-09 Darin Adler <[email protected]>
+
+ Takes two delete key presses to delete pasted emoji up-pointing index finger with skin tone
+ https://bugs.webkit.org/show_bug.cgi?id=145823
+
+ Reviewed by Anders Carlsson.
+
+ * editing/deleting/delete-emoji-expected.txt: Updated to expect a little more testing.
+ * editing/deleting/delete-emoji.html: Added a test case and streamlined the test a bit.
+
2015-06-09 Said Abou-Hallawa <[email protected]>
feComposite filter does not clip the paint rect to its effect rect when the operator is 'in' or 'atop'
Modified: trunk/LayoutTests/editing/deleting/delete-emoji-expected.txt (185392 => 185393)
--- trunk/LayoutTests/editing/deleting/delete-emoji-expected.txt 2015-06-09 23:24:12 UTC (rev 185392)
+++ trunk/LayoutTests/editing/deleting/delete-emoji-expected.txt 2015-06-09 23:49:49 UTC (rev 185393)
@@ -1,29 +1,38 @@
This test verifies that emoji groups and emoji with variations are deleted correctly
Dump of markup 1:
-| "π¦π»π¦πΎπ»π¦πΎπ¦π©βπ©βπ¦π©ββ€οΈβπ©π¨ββ€οΈβπ¨π©ββ€οΈβπβπ©π¨ββ€οΈβπβπ¨<#selection-caret>
+| "π¦π»π¦πΎπ»π¦πΎπ¦π©βπ©βπ¦π©ββ€οΈβπ©π¨ββ€οΈβπ¨π©ββ€οΈβπβπ©π¨ββ€οΈβπβπ¨βοΈπ»<#selection-caret>
"
Dump of markup 2:
+| "π¦π»π¦πΎπ»π¦πΎπ¦π©βπ©βπ¦π©ββ€οΈβπ©π¨ββ€οΈβπ¨π©ββ€οΈβπβπ©π¨ββ€οΈβπβπ¨<#selection-caret>"
+
+Dump of markup 3:
| "π¦π»π¦πΎπ»π¦πΎπ¦π©βπ©βπ¦π©ββ€οΈβπ©π¨ββ€οΈβπ¨π©ββ€οΈβπβπ©<#selection-caret>"
-Dump of markup 3:
+Dump of markup 4:
| "π¦π»π¦πΎπ»π¦πΎπ¦π©βπ©βπ¦π©ββ€οΈβπ©π¨ββ€οΈβπ¨<#selection-caret>"
-Dump of markup 4:
+Dump of markup 5:
| "π¦π»π¦πΎπ»π¦πΎπ¦π©βπ©βπ¦π©ββ€οΈβπ©<#selection-caret>"
-Dump of markup 5:
+Dump of markup 6:
| "π¦π»π¦πΎπ»π¦πΎπ¦π©βπ©βπ¦<#selection-caret>"
-Dump of markup 6:
+Dump of markup 7:
| "π¦π»π¦πΎπ»π¦πΎπ¦<#selection-caret>"
-Dump of markup 7:
+Dump of markup 8:
| "π¦π»π¦πΎπ»π¦πΎ<#selection-caret>"
-Dump of markup 8:
+Dump of markup 9:
| "π¦π»π¦πΎπ»<#selection-caret>"
-Dump of markup 9:
+Dump of markup 10:
| "π¦π»π¦πΎ<#selection-caret>"
+
+Dump of markup 11:
+| "π¦π»<#selection-caret>"
+
+Dump of markup 12:
+| <br>
Modified: trunk/LayoutTests/editing/deleting/delete-emoji.html (185392 => 185393)
--- trunk/LayoutTests/editing/deleting/delete-emoji.html 2015-06-09 23:24:12 UTC (rev 185392)
+++ trunk/LayoutTests/editing/deleting/delete-emoji.html 2015-06-09 23:49:49 UTC (rev 185393)
@@ -1,32 +1,18 @@
<!DOCTYPE html>
<html>
<body>
-<div id="test" contenteditable="true">👦🏻👦🏾🏻👦🏾👦👩‍👩‍👦👩‍❤️‍👩👨‍❤️‍👨👩‍❤️‍💋‍👩👨‍❤️‍💋‍👨
+<div id="test" contenteditable="true">👦🏻👦🏾🏻👦🏾👦👩‍👩‍👦👩‍❤️‍👩👨‍❤️‍👨👩‍❤️‍💋‍👩👨‍❤️‍💋‍👨☝️🏻
</div>
<script src=""
<script>
Markup.description("This test verifies that emoji groups and emoji with variations are deleted correctly");
-
-var selection = window.getSelection();
var testElement = document.getElementById('test');
-selection.setBaseAndExtent(testElement.firstChild, 56, testElement.firstChild, 56);
+getSelection().setBaseAndExtent(testElement.firstChild, testElement.firstChild.length, testElement.firstChild, testElement.firstChild.length);
Markup.dump("test");
-document.execCommand("Delete");
-Markup.dump("test");
-document.execCommand("Delete");
-Markup.dump("test");
-document.execCommand("Delete");
-Markup.dump("test");
-document.execCommand("Delete");
-Markup.dump("test");
-document.execCommand("Delete");
-Markup.dump("test");
-document.execCommand("Delete");
-Markup.dump("test");
-document.execCommand("Delete");
-Markup.dump("test");
-document.execCommand("Delete");
-Markup.dump("test");
+while (testElement.firstChild.length) {
+ document.execCommand("Delete");
+ Markup.dump("test");
+}
</script>
</body>
</html>
Modified: trunk/Source/WebCore/ChangeLog (185392 => 185393)
--- trunk/Source/WebCore/ChangeLog 2015-06-09 23:24:12 UTC (rev 185392)
+++ trunk/Source/WebCore/ChangeLog 2015-06-09 23:49:49 UTC (rev 185393)
@@ -1,3 +1,24 @@
+2015-06-09 Darin Adler <[email protected]>
+
+ Takes two delete key presses to delete pasted emoji up-pointing index finger with skin tone
+ https://bugs.webkit.org/show_bug.cgi?id=145823
+
+ Reviewed by Anders Carlsson.
+
+ Tests: editing/deleting/delete-emoji.html
+
+ * rendering/RenderText.cpp:
+ (WebCore::isHangulLVT): Use constants instead of macros. Also changed to take a UChar since
+ the Hangul processing can work on UTF-16 code unit at a time and doesn't have to handle
+ surrogate pairs.
+ (WebCore::isMark): Use U_GC_M_MASK instead of writing the algorithm out another way.
+ (WebCore::isInArmenianToLimbuRange): Added.
+ (WebCore::RenderText::previousOffsetForBackwardDeletion): Refactored for clarity and to use
+ the U16_PREV macro instead of doing what it does in a sloppier way. Added code to allow a
+ variation selector before an emoji modifier to fix the bug. Changed Hangul logic to work a
+ code unit at a time, since it can, to use an enum class, and to use constants rather than
+ all capital macros. Also changed the "dumb" case to use a more appropriate ICU macro.
+
2015-06-09 Said Abou-Hallawa <[email protected]>
feComposite filter does not clip the paint rect to its effect rect when the operator is 'in' or 'atop'
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (185392 => 185393)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2015-06-09 23:24:12 UTC (rev 185392)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2015-06-09 23:49:49 UTC (rev 185393)
@@ -1397,34 +1397,26 @@
#if PLATFORM(COCOA) || PLATFORM(EFL) || PLATFORM(GTK)
-#define HANGUL_CHOSEONG_START (0x1100)
-#define HANGUL_CHOSEONG_END (0x115F)
-#define HANGUL_JUNGSEONG_START (0x1160)
-#define HANGUL_JUNGSEONG_END (0x11A2)
-#define HANGUL_JONGSEONG_START (0x11A8)
-#define HANGUL_JONGSEONG_END (0x11F9)
-#define HANGUL_SYLLABLE_START (0xAC00)
-#define HANGUL_SYLLABLE_END (0xD7AF)
-#define HANGUL_JONGSEONG_COUNT (28)
+const UChar hangulChoseongStart = 0x1100;
+const UChar hangulChoseongEnd = 0x115F;
+const UChar hangulJungseongStart = 0x1160;
+const UChar hangulJungseongEnd = 0x11A2;
+const UChar hangulJongseongStart = 0x11A8;
+const UChar hangulJongseongEnd = 0x11F9;
+const UChar hangulSyllableStart = 0xAC00;
+const UChar hangulSyllableEnd = 0xD7AF;
+const UChar hangulJongseongCount = 28;
-enum HangulState {
- HangulStateL,
- HangulStateV,
- HangulStateT,
- HangulStateLV,
- HangulStateLVT,
- HangulStateBreak
-};
+enum class HangulState { L, V, T, LV, LVT, Break };
-static inline bool isHangulLVT(UChar32 character)
+static inline bool isHangulLVT(UChar character)
{
- return (character - HANGUL_SYLLABLE_START) % HANGUL_JONGSEONG_COUNT;
+ return (character - hangulSyllableStart) % hangulJongseongCount;
}
static inline bool isMark(UChar32 character)
{
- int8_t charType = u_charType(character);
- return charType == U_NON_SPACING_MARK || charType == U_ENCLOSING_MARK || charType == U_COMBINING_SPACING_MARK;
+ return U_GET_GC_MASK(character) & U_GC_M_MASK;
}
static inline bool isRegionalIndicator(UChar32 character)
@@ -1433,26 +1425,32 @@
return 0x1F1E6 <= character && character <= 0x1F1FF;
}
+static inline bool isInArmenianToLimbuRange(UChar32 character)
+{
+ return character >= 0x0530 && character < 0x1950;
+}
+
#endif
int RenderText::previousOffsetForBackwardDeletion(int current) const
{
-#if PLATFORM(COCOA) || PLATFORM(EFL) || PLATFORM(GTK)
- ASSERT(m_text);
+ ASSERT(!m_text.isNull());
StringImpl& text = *m_text.impl();
- UChar32 character;
+
+ // FIXME: Unclear why this has so much handrolled code rather than using TextBreakIterator.
+ // Also unclear why this is so different from advanceByCombiningCharacterSequence.
+
+ // FIXME: Seems like this fancier case could be used on all platforms now, no
+ // need for the #else case below.
+#if PLATFORM(COCOA) || PLATFORM(EFL) || PLATFORM(GTK)
bool sawRegionalIndicator = false;
bool sawEmojiGroupCandidate = false;
bool sawEmojiModifier = false;
while (current > 0) {
- if (U16_IS_TRAIL(text[--current]))
- --current;
- if (current < 0)
- break;
+ UChar32 character;
+ U16_PREV(text, 0, current, character);
- UChar32 character = text.characterStartingAt(current);
-
if (sawEmojiGroupCandidate) {
sawEmojiGroupCandidate = false;
if (character == zeroWidthJoiner)
@@ -1464,9 +1462,13 @@
}
if (sawEmojiModifier) {
- if (isEmojiModifier(character))
+ if (isEmojiModifier(character)) {
+ // Don't treat two emoji modifiers in a row as a group.
U16_FWD_1_UNSAFE(text, current);
- break;
+ break;
+ }
+ if (!isVariationSelector(character))
+ break;
}
if (sawRegionalIndicator) {
@@ -1480,7 +1482,7 @@
}
// We don't combine characters in Armenian ... Limbu range for backward deletion.
- if ((character >= 0x0530) && (character < 0x1950))
+ if (isInArmenianToLimbuRange(character))
break;
if (isRegionalIndicator(character)) {
@@ -1498,7 +1500,8 @@
continue;
}
- if (!isMark(character) && (character != 0xFF9E) && (character != 0xFF9F))
+ // FIXME: Why are FF9E and FF9F special cased here?
+ if (!isMark(character) && character != 0xFF9E && character != 0xFF9F)
break;
}
@@ -1506,55 +1509,50 @@
return current;
// Hangul
- character = text.characterStartingAt(current);
- if (((character >= HANGUL_CHOSEONG_START) && (character <= HANGUL_JONGSEONG_END)) || ((character >= HANGUL_SYLLABLE_START) && (character <= HANGUL_SYLLABLE_END))) {
+ UChar character = text[current];
+ if ((character >= hangulChoseongStart && character <= hangulJongseongEnd) || (character >= hangulSyllableStart && character <= hangulSyllableEnd)) {
HangulState state;
- if (character < HANGUL_JUNGSEONG_START)
- state = HangulStateL;
- else if (character < HANGUL_JONGSEONG_START)
- state = HangulStateV;
- else if (character < HANGUL_SYLLABLE_START)
- state = HangulStateT;
+ if (character < hangulJungseongStart)
+ state = HangulState::L;
+ else if (character < hangulJongseongStart)
+ state = HangulState::V;
+ else if (character < hangulSyllableStart)
+ state = HangulState::T;
else
- state = isHangulLVT(character) ? HangulStateLVT : HangulStateLV;
+ state = isHangulLVT(character) ? HangulState::LVT : HangulState::LV;
- while (current > 0 && ((character = text.characterStartingAt(current - 1)) >= HANGUL_CHOSEONG_START) && (character <= HANGUL_SYLLABLE_END) && ((character <= HANGUL_JONGSEONG_END) || (character >= HANGUL_SYLLABLE_START))) {
+ while (current > 0 && (character = text[current - 1]) >= hangulChoseongStart && character <= hangulSyllableEnd && (character <= hangulJongseongEnd || character >= hangulSyllableStart)) {
switch (state) {
- case HangulStateV:
- if (character <= HANGUL_CHOSEONG_END)
- state = HangulStateL;
- else if ((character >= HANGUL_SYLLABLE_START) && (character <= HANGUL_SYLLABLE_END) && !isHangulLVT(character))
- state = HangulStateLV;
- else if (character > HANGUL_JUNGSEONG_END)
- state = HangulStateBreak;
+ case HangulState::V:
+ if (character <= hangulChoseongEnd)
+ state = HangulState::L;
+ else if (character >= hangulSyllableStart && character <= hangulSyllableEnd && !isHangulLVT(character))
+ state = HangulState::LV;
+ else if (character > hangulJungseongEnd)
+ state = HangulState::Break;
break;
- case HangulStateT:
- if ((character >= HANGUL_JUNGSEONG_START) && (character <= HANGUL_JUNGSEONG_END))
- state = HangulStateV;
- else if ((character >= HANGUL_SYLLABLE_START) && (character <= HANGUL_SYLLABLE_END))
- state = (isHangulLVT(character) ? HangulStateLVT : HangulStateLV);
- else if (character < HANGUL_JUNGSEONG_START)
- state = HangulStateBreak;
+ case HangulState::T:
+ if (character >= hangulJungseongStart && character <= hangulJungseongEnd)
+ state = HangulState::V;
+ else if (character >= hangulSyllableStart && character <= hangulSyllableEnd)
+ state = isHangulLVT(character) ? HangulState::LVT : HangulState::LV;
+ else if (character < hangulJungseongStart)
+ state = HangulState::Break;
break;
default:
- state = (character < HANGUL_JUNGSEONG_START) ? HangulStateL : HangulStateBreak;
+ state = (character < hangulJungseongStart) ? HangulState::L : HangulState::Break;
break;
}
- if (state == HangulStateBreak)
+ if (state == HangulState::Break)
break;
-
--current;
}
}
return current;
#else
- // Platforms other than Mac delete by one code point.
- if (U16_IS_TRAIL(m_text[--current]))
- --current;
- if (current < 0)
- current = 0;
+ U16_BACK_1(text, 0, current);
return current;
#endif
}
