Bug 164595

Summary:

Composition state should be cleared when changing focus to a non-editable element

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

HTML Editing

Assignee:

Wenson Hsieh <wenson_hsieh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, commit-queue, ddkilzer, enrica, jfbastien, joepeck, rniwa, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

iPhone / iPad

OS:

iOS 10

Attachments:

Description	Flags
Begin a composition in the input, then tap the select	none
Patch	none
Rebased on ToT	none
Archive of layout-test-results from ews100 for mac-yosemite	none
Archive of layout-test-results from ews107 for mac-yosemite-wk2	none
Archive of layout-test-results from ews114 for mac-yosemite	none
Archive of layout-test-results from ews122 for ios-simulator-wk2	none
Patch	enrica: review+
Patch for landing	none

Wenson Hsieh

Reported 2016-11-10 08:50:03 PST

Created attachment 294374 [details] Begin a composition in the input, then tap the select Composition state (i.e. the composition node and underlines) are not cleared when changing focus to a non-editable element, since selection is None at that point.

Attachments
Begin a composition in the input, then tap the select (94.77 KB, application/zip) 2016-11-10 08:50 PST, Wenson Hsieh	no flags	Details
Patch (60.40 KB, patch) 2016-11-10 09:32 PST, Wenson Hsieh	no flags	Details Formatted Diff Diff
Rebased on ToT (60.38 KB, patch) 2016-11-10 09:37 PST, Wenson Hsieh	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews100 for mac-yosemite (935.99 KB, application/zip) 2016-11-10 10:37 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.05 MB, application/zip) 2016-11-10 10:42 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews114 for mac-yosemite (1.69 MB, application/zip) 2016-11-10 10:59 PST, Build Bot	no flags	Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (7.16 MB, application/zip) 2016-11-10 11:05 PST, Build Bot	no flags	Details
Patch (67.42 KB, patch) 2016-11-10 13:22 PST, Wenson Hsieh	enrica: review+	Details Formatted Diff Diff
Patch for landing (67.51 KB, patch) 2016-11-11 10:04 PST, Wenson Hsieh	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Wenson Hsieh

Comment 1 2016-11-10 09:32:55 PST

Created attachment 294379 [details] Patch

Wenson Hsieh

Comment 2 2016-11-10 09:33:48 PST

<rdar://problem/26412551>

Wenson Hsieh

Comment 3 2016-11-10 09:37:05 PST

Created attachment 294380 [details] Rebased on ToT

Build Bot

Comment 4 2016-11-10 10:37:30 PST

Comment on attachment 294380 [details] Rebased on ToT Attachment 294380 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2491681 New failing tests: editing/focus-change-with-marked-text.html

Build Bot

Comment 5 2016-11-10 10:37:33 PST

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

Build Bot

Comment 6 2016-11-10 10:42:06 PST

Comment on attachment 294380 [details] Rebased on ToT Attachment 294380 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2491701 New failing tests: editing/focus-change-with-marked-text.html

Build Bot

Comment 7 2016-11-10 10:42:09 PST

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

David Kilzer (:ddkilzer)

Comment 8 2016-11-10 10:44:42 PST

Comment on attachment 294380 [details] Rebased on ToT View in context: https://bugs.webkit.org/attachment.cgi?id=294380&action=review > Tools/ChangeLog:34 > + * DumpRenderTree/mac/TextInputControllerMac.m: Renamed from Tools/DumpRenderTree/mac/TextInputController.m. Technically, this should go in /cocoa/ folder instead of /mac/, but that doesn't have to be done in this patch. > Tools/DumpRenderTree/TextInputControllerIOS.m:2 > + * Copyright (C) 2016 Apple Inc. All rights reserved. This should go in /ios/ folder.

Build Bot

Comment 9 2016-11-10 10:59:38 PST

Comment on attachment 294380 [details] Rebased on ToT Attachment 294380 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2491748 New failing tests: editing/focus-change-with-marked-text.html storage/domstorage/sessionstorage/blocked-file-access.html

Build Bot

Comment 10 2016-11-10 10:59:41 PST

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

Build Bot

Comment 11 2016-11-10 11:05:00 PST

Comment on attachment 294380 [details] Rebased on ToT Attachment 294380 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2491732 New failing tests: editing/focus-change-with-marked-text.html

Build Bot

Comment 12 2016-11-10 11:05:03 PST

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

Wenson Hsieh

Comment 13 2016-11-10 12:20:05 PST

Comment on attachment 294380 [details] Rebased on ToT View in context: https://bugs.webkit.org/attachment.cgi?id=294380&action=review I'm also looking into why these tests fail on Mac. It's likely the semantics of -setMarkedText: and -unmarkText are just different across the two platforms, so we might need to disable this on Mac. >> Tools/DumpRenderTree/TextInputControllerIOS.m:2 >> + * Copyright (C) 2016 Apple Inc. All rights reserved. > > This should go in /ios/ folder. Good catch! Moved to ios/

Wenson Hsieh

Comment 14 2016-11-10 13:22:54 PST

Created attachment 294404 [details] Patch

Enrica Casucci

Comment 15 2016-11-11 09:38:50 PST

Comment on attachment 294404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294404&action=review This is awesome! > Tools/DumpRenderTree/TextInputController.h:2 > + * Copyright (C) 2005 Apple Inc. All rights reserved. Fix copyright year. > Tools/DumpRenderTree/TextInputController.h:31 > +// FIXME: <rdar://problem/5106287> DumpRenderTree: fix TextInputController to work with iOS and re-enable tests Is this radar now fixed? If so you should move it to integrate and reference it in the change log as well. Remove or update this comment. > Tools/DumpRenderTree/mac/TextInputControllerMac.m:2 > + * Copyright (C) 2005, 2007 Apple Inc. All rights reserved. Update copyright year

Wenson Hsieh

Comment 16 2016-11-11 09:47:50 PST

Comment on attachment 294404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=294404&action=review >> Tools/DumpRenderTree/TextInputController.h:2 >> + * Copyright (C) 2005 Apple Inc. All rights reserved. > > Fix copyright year. Good catch! Fixed. >> Tools/DumpRenderTree/TextInputController.h:31 >> +// FIXME: <rdar://problem/5106287> DumpRenderTree: fix TextInputController to work with iOS and re-enable tests > > Is this radar now fixed? If so you should move it to integrate and reference it in the change log as well. Remove or update this comment. Updated the comment -- there's still work to be done, since TextInputController on iOS isn't still up to parity with Mac (there are many commands that need to be implemented) but this is definitely progress towards that bug. I'll comment in <rdar://problem/5106287> as well. >> Tools/DumpRenderTree/mac/TextInputControllerMac.m:2 >> + * Copyright (C) 2005, 2007 Apple Inc. All rights reserved. > > Update copyright year Fixed!

Wenson Hsieh

Comment 17 2016-11-11 10:04:37 PST

Created attachment 294504 [details] Patch for landing

WebKit Commit Bot

Comment 18 2016-11-11 10:41:07 PST

Comment on attachment 294504 [details] Patch for landing Clearing flags on attachment: 294504 Committed r208593: <http://trac.webkit.org/changeset/208593>

JF Bastien

Comment 19 2016-11-13 10:58:55 PST

FYI this broke the CMake build, I'm fixing the TextInputControllerMac.m rename here: https://bugs.webkit.org/show_bug.cgi?id=164526 I haven't checked whether there's further breakage of CMake. Simple repro: mkdir out xcrun cmake -Wno-dev .. -G Ninja \ -DCMAKE_BUILD_TYPE=Release \ -DPORT=Mac ninja TestWTF

Wenson Hsieh

Comment 20 2016-11-13 11:32:00 PST

(In reply to comment #19) > FYI this broke the CMake build, I'm fixing the TextInputControllerMac.m > rename here: > https://bugs.webkit.org/show_bug.cgi?id=164526 > > I haven't checked whether there's further breakage of CMake. > > Simple repro: > > mkdir out > xcrun cmake -Wno-dev .. -G Ninja \ > -DCMAKE_BUILD_TYPE=Release \ > -DPORT=Mac > ninja TestWTF Sorry about that -- I forgot to update the cmake file during my refactoring. Thank you for fixing this!

JF Bastien

Comment 21 2016-11-13 18:10:12 PST

(In reply to comment #20) > (In reply to comment #19) > > FYI this broke the CMake build, I'm fixing the TextInputControllerMac.m > > rename here: > > https://bugs.webkit.org/show_bug.cgi?id=164526 > > > > I haven't checked whether there's further breakage of CMake. > > > > Simple repro: > > > > mkdir out > > xcrun cmake -Wno-dev .. -G Ninja \ > > -DCMAKE_BUILD_TYPE=Release \ > > -DPORT=Mac > > ninja TestWTF > > Sorry about that -- I forgot to update the cmake file during my refactoring. > Thank you for fixing this! No worries, it was an easy fix!

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