Bug 178096 - Delete button doesn't fully delete certain emoji
Summary: Delete button doesn't fully delete certain emoji
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-09 14:29 PDT by Myles C. Maxfield
Modified: 2017-10-17 14:21 PDT (History)
11 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2017-10-09 14:29:34 PDT
Delete button doesn't fully delete certain emoji
Comment 1 Myles C. Maxfield 2017-10-09 14:31:45 PDT
Created attachment 323223 [details]
Patch
Comment 2 Myles C. Maxfield 2017-10-09 14:32:10 PDT
Created attachment 323224 [details]
Patch
Comment 3 Myles C. Maxfield 2017-10-09 14:32:48 PDT
<rdar://problem/34785106>
Comment 4 Myles C. Maxfield 2017-10-09 15:16:25 PDT
Created attachment 323232 [details]
Patch
Comment 5 Simon Fraser (smfr) 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?
Comment 6 Myles C. Maxfield 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)
Comment 7 Darin Adler 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?
Comment 8 Myles C. Maxfield 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! 😎
Comment 9 Build Bot 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
Comment 10 Build Bot 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
Comment 11 Myles C. Maxfield 2017-10-09 18:15:47 PDT
Committed r223110: <http://trac.webkit.org/changeset/223110>
Comment 13 Matt Lewis 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.
Comment 14 Matt Lewis 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>
Comment 15 Myles C. Maxfield 2017-10-10 13:35:53 PDT
Created attachment 323340 [details]
WIP
Comment 16 Myles C. Maxfield 2017-10-11 15:44:36 PDT
Created attachment 323475 [details]
test
Comment 17 Myles C. Maxfield 2017-10-11 16:31:22 PDT
Created attachment 323484 [details]
WIP
Comment 18 Myles C. Maxfield 2017-10-12 14:16:52 PDT
Created attachment 323563 [details]
WIP
Comment 19 Myles C. Maxfield 2017-10-12 16:14:57 PDT
Created attachment 323590 [details]
New expected results
Comment 20 Build Bot 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
Comment 21 Build Bot 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
Comment 22 Build Bot 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
Comment 23 Build Bot 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
Comment 24 Build Bot 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
Comment 25 Build Bot 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
Comment 26 Myles C. Maxfield 2017-10-13 11:35:58 PDT
Created attachment 323712 [details]
WIP
Comment 27 Build Bot 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
Comment 28 Build Bot 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
Comment 29 Build Bot 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
Comment 30 Build Bot 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
Comment 31 Build Bot 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
Comment 32 Build Bot 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
Comment 33 Myles C. Maxfield 2017-10-13 14:10:14 PDT
Created attachment 323743 [details]
WIP
Comment 34 Myles C. Maxfield 2017-10-13 14:38:31 PDT
Created attachment 323750 [details]
Patch
Comment 35 Build Bot 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
Comment 36 Build Bot 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
Comment 37 Myles C. Maxfield 2017-10-17 12:39:56 PDT
Created attachment 324042 [details]
Patch for committing
Comment 38 WebKit Commit Bot 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>