Issue metadata
Sign in to add a comment
|
Switching between emoji in contenteditable div adds garbage characters
Reported by
ndo...@twitter.com,
Mar 28 2017
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.110 Safari/537.36 Steps to reproduce the problem: Repro at https://jsfiddle.net/kbd1rq8v/2/ Minimal HTML is also attached 1. Create a contenteditable div including an anchor 2. Use the Japanese Hiragana keyboard to add emoji after the anchor in that div 3. Switch from one emoji to another What is the expected behavior? Switching between emoji in the input dropdown should switch between emoji within the editable div. It should also not add garbage characters What went wrong? When you go from one emoji candidate to another, the new emoji is not added to the document. Instead, the emoji remains the same and "�" characters are added after it. Did this work before? Yes Chrome 56 Chrome version: 57.0.2987.110 Channel: stable OS Version: OS X 10.11.6 Flash Version: This behavior doesn't exist in Firefox 52 or Safari 10.1
,
Mar 29 2017
Multiple code units character, e.g. U+1F408, causes this.
,
Mar 29 2017
InsertText command works as expected: https://jsfiddle.net/hx4efaa0/
,
Mar 29 2017
The root cause is computeCommonPrefixLength() doesn't consider multiple code unit code point.
computeCommonPrefixLength("\uD83D\uDE3A", "\uD83D\uDE38"), it should return 0 instead of 1.
"\uD83D\uDE3A" == U+1F63A
"\uD83D\uDE38" == U+1F638
,
Mar 29 2017
Here is trace:
insertTextRunWithoutNewlines $[2] L"😸" $true $TextCompositionUpdate (1)
computeCommonPrefixLength: old=L"🐈" new=L"😸" -> 1
computeDistanceToRightGraphemeBoundary: {m_anchorNode=B🐈 m_offset=1 m_anchorType=OffsetInAnchor (0) } -> {m_anchorNode=B🐈 m_offset=1 m_anchorType=OffsetInAnchor (0) }
InsertIncrementalTextCommand::doApply [2] L"😸", commonPrefixLength=1, commonSuffixLength=0
computeSelectionForInsertion: offset=1, length=1 start=2 end=2 # end of existing string
nsertIncrementalTextCommand::doApply: m_text=$[1] L"☐" ${m_base={m_anchorNode=B🐈 m_offset=3 m_anchorType=OffsetInAnchor (0) } m_extent={m_anchorNode=B🐈 m_offset=...} ...}
adjustSelectionAfterIncrementalInsertion: ${m_base={m_anchorNode=B🐈☐ m_offset=1 m_anchorType=OffsetInAnchor (0) } m_extent={m_anchorNode=B🐈☐ ...} ...}
,
Mar 29 2017
,
Apr 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1cc2c7d24ced1b58e047a4dea8a78493e32a1bf0 commit 1cc2c7d24ced1b58e047a4dea8a78493e32a1bf0 Author: yosin <yosin@chromium.org> Date: Mon Apr 03 07:32:09 2017 Make InsertIncrementalTextCommand to handle surrogate pair properly This patch makes |InsertIncrementalTextCommand| to handle surrogate pairs properly. Before this patch, |InsertIncrementalTextCommand| to attempt to replace trailing surrogate pairs code unit with surrogate pair then produces surrogate pairs + trailing surrogate pair. Example: Old text: replace D83D DE3A (U+1F63A) by D83D DE38 (U+1F638) Current behavior: D83D DE3A DE38 New behavior: D83D DE38 This is cause by computeCommonPrefixLength(oldString, newString) returns 1 in above example where middle of surrogate pairs. BUG= 706166 TEST=run_webkit_unit_tests --gtest_filter=InsertIncrementalTextCommandTest.SurrogatePairs* Review-Url: https://codereview.chromium.org/2782823002 Cr-Commit-Position: refs/heads/master@{#461382} [modify] https://crrev.com/1cc2c7d24ced1b58e047a4dea8a78493e32a1bf0/third_party/WebKit/Source/core/editing/BUILD.gn [modify] https://crrev.com/1cc2c7d24ced1b58e047a4dea8a78493e32a1bf0/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/1cc2c7d24ced1b58e047a4dea8a78493e32a1bf0/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.h [add] https://crrev.com/1cc2c7d24ced1b58e047a4dea8a78493e32a1bf0/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommandTest.cpp [modify] https://crrev.com/1cc2c7d24ced1b58e047a4dea8a78493e32a1bf0/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.h
,
Apr 11 2017
What does the release path look like for this bug? Should we expect to continue to see this bug on Chrome versions 57 & 58 (and thus need to leave workarounds in place until those versions phase out)?
,
May 24 2017
Sorry for later response. This fix will be in M59.
,
May 24 2017
Got it. Thanks! |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by patricia...@chromium.org
, Mar 28 2017Status: Untriaged (was: Unconfirmed)