Bug 187554

Summary: [iOS] WKWebView should match UITextView behavior when editing text with an RTL keyboard
Product: WebKit Reporter: mitz
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, darin, ews-watchlist, megan_gardner, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
ews-watchlist: commit-queue-
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
First pass
none
Patch for EWS
wenson_hsieh: commit-queue+
Patch for landing none

mitz
Reported 2018-07-11 09:42:49 PDT
In UIWebView, when the insertion point is inside an empty block in editable content, switching keyboards causes the dir attribute of the block to be set to match the writing direction associated with the keyboard. In WKWebView this isn’t happening.
Attachments
WIP (46.77 KB, patch)
2018-12-05 22:52 PST, Wenson Hsieh 		ews-watchlist: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.37 MB, application/zip)
2018-12-06 00:44 PST, EWS Watchlist 		no flags
Details
First pass (39.60 KB, patch)
2018-12-06 09:52 PST, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
Patch for EWS (40.94 KB, patch)
2018-12-06 11:25 PST, Wenson Hsieh 		wenson_hsieh: commit-queue+
DetailsFormatted DiffDiff
Patch for landing (41.82 KB, patch)
2018-12-06 12:52 PST, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
mitz
Comment 1 2018-07-11 09:44:11 PDT
<rdar://problem/42075638>
Wenson Hsieh
Comment 2 2018-12-04 21:09:21 PST
*** Bug 192393 has been marked as a duplicate of this bug. ***
Wenson Hsieh
Comment 3 2018-12-05 22:52:55 PST Comment hidden (obsolete)
EWS Watchlist
Comment 4 2018-12-06 00:44:24 PST Comment hidden (obsolete)
EWS Watchlist
Comment 5 2018-12-06 00:44:25 PST Comment hidden (obsolete)
Wenson Hsieh
Comment 6 2018-12-06 09:52:21 PST
Created attachment 356736 [details] First pass
Tim Horton
Comment 7 2018-12-06 11:04:11 PST
Comment on attachment 356736 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=356736&action=review > Source/WebKit/UIProcess/WebPageProxy.cpp:1875 > + if (isValid()) This is a surprising and bizarre pattern! Would much prefer if these were all early returns, despite being more lines. > Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:911 > + m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = false; Do we get here if the process dies? (otherwise, you might need to reset this there too? > Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:176 > + > + if (options.editable) > + m_mainWebView->setEditable(true); How did we not have this before
Wenson Hsieh
Comment 8 2018-12-06 11:25:28 PST
Created attachment 356738 [details] Patch for EWS
Wenson Hsieh
Comment 9 2018-12-06 11:25:44 PST
Comment on attachment 356736 [details] First pass View in context: https://bugs.webkit.org/attachment.cgi?id=356736&action=review >> Source/WebKit/UIProcess/WebPageProxy.cpp:1875 >> + if (isValid()) > > This is a surprising and bizarre pattern! Would much prefer if these were all early returns, despite being more lines. Ok! I'll change these to early returns. >> Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:911 >> + m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = false; > > Do we get here if the process dies? (otherwise, you might need to reset this there too? Good catch — yes, this flag (as well as m_deferredNodeAssistanceArguments) should both be reset! >> Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:176 >> + m_mainWebView->setEditable(true); > > How did we not have this before 🤷🏻‍♂️
Darin Adler
Comment 10 2018-12-06 12:33:55 PST
Comment on attachment 356738 [details] Patch for EWS View in context: https://bugs.webkit.org/attachment.cgi?id=356738&action=review > Source/WebKit/UIProcess/WebPageProxy.h:87 > +#include <WebCore/WritingDirection.h> Should be able to use forward declaration instead of include in this case. > Source/WebKit/WebProcess/WebPage/WebPage.h:73 > +#include <WebCore/WritingDirection.h> Ditto.
Wenson Hsieh
Comment 11 2018-12-06 12:45:06 PST
(In reply to Darin Adler from comment #10) > Comment on attachment 356738 [details] > Patch for EWS > > View in context: > https://bugs.webkit.org/attachment.cgi?id=356738&action=review > > > Source/WebKit/UIProcess/WebPageProxy.h:87 > > +#include <WebCore/WritingDirection.h> > > Should be able to use forward declaration instead of include in this case. > > > Source/WebKit/WebProcess/WebPage/WebPage.h:73 > > +#include <WebCore/WritingDirection.h> > > Ditto. 👍 will make these forward declarations before landing.
Wenson Hsieh
Comment 12 2018-12-06 12:52:52 PST
Created attachment 356741 [details] Patch for landing
WebKit Commit Bot
Comment 13 2018-12-06 13:21:30 PST
The commit-queue encountered the following flaky tests while processing attachment 356741 [details]: workers/bomb.html bug 192050 (author: fpizlo@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 14 2018-12-06 13:22:28 PST
Comment on attachment 356741 [details] Patch for landing Clearing flags on attachment: 356741 Committed r238939: <https://trac.webkit.org/changeset/238939>
WebKit Commit Bot
Comment 15 2018-12-06 13:22:30 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.