|Summary:||WebKit2: Text input is largely broken when there are subframes loading|
|Product:||WebKit||Reporter:||Alexey Proskuryakov <ap>|
|Component:||HTML Editing||Assignee:||Alexey Proskuryakov <ap>|
|Severity:||Normal||CC:||beidson, commit-queue, darin, igor.oliveira, japhet, naofumi|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.6|
Description Alexey Proskuryakov 2011-04-21 11:18:07 PDT
We reset text input state on frame load, failing to recognize that there is only one WKView per page, but multiple frames. Steps to reproduce: 1. Open attach test case. 2. Try to type and edit text using any non-trivial features (inline input, dictionary pop-up, possibly accessibility). Observe that it's obviously and mightily broken.
Comment 2 Alexey Proskuryakov 2011-04-21 15:24:42 PDT
Created attachment 90614 [details] test case (open from a local file)
Comment 3 Alexey Proskuryakov 2011-08-29 00:36:32 PDT
*** Bug 67105 has been marked as a duplicate of this bug. ***
Comment 4 Alexey Proskuryakov 2013-05-16 18:16:57 PDT
Created attachment 202008 [details] proposed fix
Comment 5 Brady Eidson 2013-05-17 09:20:45 PDT
Comment on attachment 202008 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=202008&action=review > Source/WebCore/ChangeLog:20 > + Added a hook taht gets invoked right before transitioning to committed state starts. s/taht/that/ > LayoutTests/platform/mac-wk2/TestExpectations:321 > +# textInputController.hasMarkedText is implemented, but gives a wrong result for some reason. > +platform/mac/editing/input/unconfirmed-text-navigation-with-page-cache.html Kind of troubling that the test doesn't cover the port where the most changes took place. =/
Comment 6 Alexey Proskuryakov 2013-05-17 09:45:35 PDT
Comment on attachment 202008 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=202008&action=review >> LayoutTests/platform/mac-wk2/TestExpectations:321 >> +platform/mac/editing/input/unconfirmed-text-navigation-with-page-cache.html > > Kind of troubling that the test doesn't cover the port where the most changes took place. =/ Currently, many text input tests are failing or skipped because or WTR deficiencies, and there are many aspects of text input that can't be tested on any platform. It's unfortunate that text input test coverage is so spotty, but I guess it's better than nothing. In WebKit2, driving tests from WebProcess makes it particularly tricky to test anything that deals with user actions.
Comment 7 Darin Adler 2013-05-17 10:16:11 PDT
Comment on attachment 202008 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=202008&action=review review+ as long as you do not add a member function to Frame! > Source/WebCore/page/Frame.h:110 > + void willTransitionToCommitted(); We should not add a function to Frame for this. The code should be in FrameLoader. It’s completely reasonable for FrameLoader to make Editor calls. You can put this new function in either FrameLoader or Editor, but not in Frame. > Source/WebKit2/UIProcess/WebPageProxy.cpp:3070 > + // This is a temporary state when editing. Flipping secure input state too quickly can expose race conditions. > + if (couldChangeSecureInputState && !editorState.selectionIsNone) > + m_pageClient->updateSecureInputState(); I don’t understand the comment. It doesn’t say what it’s referring to. What is a temporary state when editing? What does “too quickly” mean? It seems like you moved this comment here, but I don’t understand it in its original location.