Bug 178096

Summary:

Delete button doesn't fully delete certain emoji

Product:

WebKit

Reporter:

Myles C. Maxfield <mmaxfield>

Component:

New Bugs

Assignee:

Myles C. Maxfield <mmaxfield>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ahmad.saleem792, buildbot, commit-queue, darin, dino, jlewis3, jonlee, rniwa, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	simon.fraser: review+, buildbot: commit-queue-
Archive of layout-test-results from ews121 for ios-simulator-wk2	none
Failed Diff	none
WIP	none
test	none
WIP	none
WIP	none
New expected results	buildbot: commit-queue-
Archive of layout-test-results from ews115 for mac-elcapitan	none
Archive of layout-test-results from ews103 for mac-elcapitan	none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2	none
WIP	buildbot: commit-queue-
Archive of layout-test-results from ews107 for mac-elcapitan-wk2	none
Archive of layout-test-results from ews100 for mac-elcapitan	none
Archive of layout-test-results from ews116 for mac-elcapitan	none
WIP	none
Patch	buildbot: commit-queue-
Archive of layout-test-results from ews126 for ios-simulator-wk2	none
Patch for committing	none

Myles C. Maxfield

Reported 2017-10-09 14:29:34 PDT

Delete button doesn't fully delete certain emoji

Attachments
Patch (165.47 KB, patch) 2017-10-09 14:31 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (165.54 KB, patch) 2017-10-09 14:32 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (155.69 KB, patch) 2017-10-09 15:16 PDT, Myles C. Maxfield	simon.fraser: review+ buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (1.16 MB, application/zip) 2017-10-09 17:40 PDT, Build Bot	no flags	Details
Failed Diff (80.50 KB, text/plain) 2017-10-10 10:26 PDT, Matt Lewis	no flags	Details
WIP (160.27 KB, patch) 2017-10-10 13:35 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
test (1.82 KB, text/html) 2017-10-11 15:44 PDT, Myles C. Maxfield	no flags	Details
WIP (207.39 KB, patch) 2017-10-11 16:31 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
WIP (303.31 KB, patch) 2017-10-12 14:16 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
New expected results (345.22 KB, patch) 2017-10-12 16:14 PDT, Myles C. Maxfield	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews115 for mac-elcapitan (2.02 MB, application/zip) 2017-10-12 17:55 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews103 for mac-elcapitan (1021.47 KB, application/zip) 2017-10-12 18:59 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.42 MB, application/zip) 2017-10-12 19:21 PDT, Build Bot	no flags	Details
WIP (327.44 KB, patch) 2017-10-13 11:35 PDT, Myles C. Maxfield	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (1.54 MB, application/zip) 2017-10-13 13:06 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews100 for mac-elcapitan (1.13 MB, application/zip) 2017-10-13 13:27 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews116 for mac-elcapitan (2.09 MB, application/zip) 2017-10-13 13:35 PDT, Build Bot	no flags	Details
WIP (304.15 KB, patch) 2017-10-13 14:10 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Patch (262.35 KB, patch) 2017-10-13 14:38 PDT, Myles C. Maxfield	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (975.24 KB, application/zip) 2017-10-13 18:24 PDT, Build Bot	no flags	Details
Patch for committing (266.22 KB, patch) 2017-10-17 12:39 PDT, Myles C. Maxfield	no flags	Details Formatted Diff Diff
Show Obsolete (19) View All Add attachment proposed patch, testcase, etc.

Myles C. Maxfield

Comment 1 2017-10-09 14:31:45 PDT

Created attachment 323223 [details] Patch

Myles C. Maxfield

Comment 2 2017-10-09 14:32:10 PDT

Created attachment 323224 [details] Patch

Myles C. Maxfield

Comment 3 2017-10-09 14:32:48 PDT

<rdar://problem/34785106>

Myles C. Maxfield

Comment 4 2017-10-09 15:16:25 PDT

Created attachment 323232 [details] Patch

Simon Fraser (smfr)

Comment 5 2017-10-09 15:49:21 PDT

Comment on attachment 323232 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323232&action=review > Source/WebCore/ChangeLog:11 > + System infrastructure for handling emoji changes every year. Instead of having > + custom code to specifically walk over codepoints, we should delegate to the > + system handling. Can we do this on all systems that we support?

Myles C. Maxfield

Comment 6 2017-10-09 15:52:10 PDT

Comment on attachment 323232 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323232&action=review >> Source/WebCore/ChangeLog:11 >> + system handling. > > Can we do this on all systems that we support? Yep, it falls back to ICU on non-cocoa platforms (and all ports support ICU)

Darin Adler

Comment 7 2017-10-09 16:10:08 PDT

Comment on attachment 323232 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323232&action=review > Source/WebCore/rendering/RenderText.cpp:1533 > + CachedTextBreakIterator iterator(StringView(m_text), TextBreakIterator::Mode::Caret, nullAtom()); Do we need to explicitly cast to StringView?

Myles C. Maxfield

Comment 8 2017-10-09 16:35:44 PDT

Comment on attachment 323232 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=323232&action=review >> Source/WebCore/rendering/RenderText.cpp:1533 >> + CachedTextBreakIterator iterator(StringView(m_text), TextBreakIterator::Mode::Caret, nullAtom()); > > Do we need to explicitly cast to StringView? Nope! 😎

Build Bot

Comment 9 2017-10-09 17:40:37 PDT

Comment on attachment 323232 [details] Patch Attachment 323232 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4805921 New failing tests: editing/deleting/delete-emoji-2.html editing/deleting/delete-emoji.html

Build Bot

Comment 10 2017-10-09 17:40:38 PDT

Created attachment 323256 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6

Myles C. Maxfield

Comment 11 2017-10-09 18:15:47 PDT

Committed r223110: <http://trac.webkit.org/changeset/223110>

Matt Lewis

Comment 12 2017-10-10 10:24:11 PDT

This revision caused the test editing/deleting/delete-emoji.html to start failing consistently on macOS Sierra and timeout consistently on macOS El Capitan: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=editing%2Fdeleting%2Fdelete-emoji.html Fail: https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20(Tests)/builds/4930 Timeout: https://build.webkit.org/builders/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/builds/3555

Matt Lewis

Comment 13 2017-10-10 10:26:32 PDT

Created attachment 323315 [details] Failed Diff This is the diff for the failure on Sierra WK2 Release. If this cant be resolved soon, I'd like to revert the change as it will begin causing problems in EWS.

Matt Lewis

Comment 14 2017-10-10 10:58:58 PDT

Reverted r223110 for reason: This caused consistent failures and timeouts on multiple platforms. Committed r223134: <http://trac.webkit.org/changeset/223134>

Myles C. Maxfield

Comment 15 2017-10-10 13:35:53 PDT

Created attachment 323340 [details] WIP

Myles C. Maxfield

Comment 16 2017-10-11 15:44:36 PDT

Created attachment 323475 [details] test

Myles C. Maxfield

Comment 17 2017-10-11 16:31:22 PDT

Created attachment 323484 [details] WIP

Myles C. Maxfield

Comment 18 2017-10-12 14:16:52 PDT

Created attachment 323563 [details] WIP

Myles C. Maxfield

Comment 19 2017-10-12 16:14:57 PDT

Created attachment 323590 [details] New expected results

Build Bot

Comment 20 2017-10-12 17:55:11 PDT

Comment on attachment 323590 [details] New expected results Attachment 323590 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4838962 New failing tests: editing/deleting/delete-emoji-3.html editing/deleting/delete-emoji-2.html editing/deleting/delete-emoji.html

Build Bot

Comment 21 2017-10-12 17:55:13 PDT

Created attachment 323613 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 22 2017-10-12 18:59:34 PDT

Comment on attachment 323590 [details] New expected results Attachment 323590 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4839860 New failing tests: editing/deleting/delete-emoji-3.html editing/deleting/delete-emoji-2.html editing/deleting/delete-emoji.html

Build Bot

Comment 23 2017-10-12 18:59:35 PDT

Created attachment 323621 [details] Archive of layout-test-results from ews103 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 24 2017-10-12 19:21:07 PDT

Comment on attachment 323590 [details] New expected results Attachment 323590 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4840059 New failing tests: editing/deleting/delete-emoji-3.html editing/deleting/delete-emoji-2.html editing/deleting/delete-emoji.html

Build Bot

Comment 25 2017-10-12 19:21:09 PDT

Created attachment 323622 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Myles C. Maxfield

Comment 26 2017-10-13 11:35:58 PDT

Created attachment 323712 [details] WIP

Build Bot

Comment 27 2017-10-13 13:06:11 PDT

Comment on attachment 323712 [details] WIP Attachment 323712 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4849255 New failing tests: editing/deleting/delete-emoji-9.html editing/deleting/delete-emoji-8.html editing/deleting/delete-emoji-6.html editing/deleting/delete-emoji.html editing/deleting/delete-emoji-4.html editing/deleting/delete-emoji-2.html editing/deleting/delete-emoji-3.html editing/deleting/delete-emoji-5.html editing/deleting/delete-emoji-7.html

Build Bot

Comment 28 2017-10-13 13:06:12 PDT

Created attachment 323730 [details] Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6

Build Bot

Comment 29 2017-10-13 13:27:37 PDT

Comment on attachment 323712 [details] WIP Attachment 323712 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4849535 New failing tests: editing/deleting/delete-emoji-9.html editing/deleting/delete-emoji-8.html editing/deleting/delete-emoji-6.html editing/deleting/delete-emoji.html editing/deleting/delete-emoji-4.html editing/deleting/delete-emoji-2.html editing/deleting/delete-emoji-3.html editing/deleting/delete-emoji-5.html editing/deleting/delete-emoji-7.html

Build Bot

Comment 30 2017-10-13 13:27:38 PDT

Created attachment 323735 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Build Bot

Comment 31 2017-10-13 13:35:08 PDT

Comment on attachment 323712 [details] WIP Attachment 323712 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4849432 New failing tests: editing/deleting/delete-emoji-9.html editing/deleting/delete-emoji-8.html editing/deleting/delete-emoji-6.html editing/deleting/delete-emoji.html editing/deleting/delete-emoji-4.html editing/deleting/delete-emoji-2.html editing/deleting/delete-emoji-3.html editing/deleting/delete-emoji-5.html editing/deleting/delete-emoji-7.html

Build Bot

Comment 32 2017-10-13 13:35:10 PDT

Created attachment 323737 [details] Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Myles C. Maxfield

Comment 33 2017-10-13 14:10:14 PDT

Created attachment 323743 [details] WIP

Myles C. Maxfield

Comment 34 2017-10-13 14:38:31 PDT

Created attachment 323750 [details] Patch

Build Bot

Comment 35 2017-10-13 18:24:30 PDT

Comment on attachment 323750 [details] Patch Attachment 323750 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4852668 New failing tests: editing/deleting/delete-emoji-9.html

Build Bot

Comment 36 2017-10-13 18:24:32 PDT

Created attachment 323783 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6

Myles C. Maxfield

Comment 37 2017-10-17 12:39:56 PDT

Created attachment 324042 [details] Patch for committing

WebKit Commit Bot

Comment 38 2017-10-17 14:21:37 PDT

Comment on attachment 324042 [details] Patch for committing Clearing flags on attachment: 324042 Committed r223578: <https://trac.webkit.org/changeset/223578>

Ahmad Saleem

Comment 39 2022-09-24 10:04:09 PDT

Landed - https://github.com/WebKit/WebKit/commit/7ece327caf929743ca3eab0654c3fcaf8609e324 https://github.com/WebKit/WebKit/commit/28d78645e97862fb0b4bbafbc436c3a7a023e424 but seems to be backed-out: https://github.com/WebKit/WebKit/commit/14819586c2767f05a240e49553ce084a843ba58a Is this still required?

Note You need to log in before you can comment on or make changes to this bug.