Commit e042b63d authored by ap@apple.com's avatar ap@apple.com

Text input is largely broken when there are subframes loading

        http://bugs.webkit.org/show_bug.cgi?id=59121
        <rdar://problem/9320468>

        Reviewed by Darin Adler.

        * UIProcess/PageClient.h:
        * UIProcess/API/mac/PageClientImpl.h:
        * UIProcess/API/mac/PageClientImpl.mm:
        (WebKit::PageClientImpl::updateSecureInputState): Separated secure input state
        updating into a separate function. Removed updateTextInputState, we don't need
        to go through PageClient to implement its behavior at all.
        (WebKit::PageClientImpl::dismissDictionaryLookupPanel): Added a FIXME.

        * UIProcess/API/mac/WKView.mm:
        * UIProcess/API/mac/WKViewInternal.h:
        Removed _updateTextInputStateIncludingSecureInputState.

        * UIProcess/WebPageProxy.h: Added m_temporarilyClosedComposition, which helps
        to figure out that WebCore decided to close a composition. The issue is that WebCore
        would first send an EditorState with hasComposition set to false, and with
        shouldIgnoreCompositionSelectionChange set to true, at which time we forget the
        previous m_editorState, but can't make any decisions based on this transient state.
        We should find a way to simplify this (maybe not send these updates with
        shouldIgnoreCompositionSelectionChange at all?)

        * UIProcess/WebPageProxy.cpp:
        (WebKit::WebPageProxy::WebPageProxy): Initialize m_temporarilyClosedComposition.
        (WebKit::WebPageProxy::didCommitLoadForFrame): Removed the code to kill a composition
        when any frame commits a load, which made no sense (along with surrounding code,
        which will unfortunately survive longer).
        (WebKit::WebPageProxy::editorStateChanged): Implemented state updating here,
        we don't need to go to WKView.mm to implement this logic. Figure out when WebCore
        discards a composition, and notify input methods about this.
        (WebKit::WebPageProxy::resetStateAfterProcessExited): Reset m_temporarilyClosedComposition.
        Added some FIXMEs.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@150291 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 12a72937
2013-05-16 Alexey Proskuryakov <ap@apple.com>
Text input is largely broken when there are subframes loading
http://bugs.webkit.org/show_bug.cgi?id=59121
<rdar://problem/9320468>
Reviewed by Darin Adler.
The new test's result are affected by DRT and WTR deficiencies, but it still
verifies that a tricky crash I had during development doesn't start to happen again.
* platform/mac-wk2/TestExpectations:
* platform/mac/editing/input/resources: Added.
* platform/mac/editing/input/resources/first-page.html: Added.
* platform/mac/editing/input/resources/other-page.html: Added.
* platform/mac/editing/input/unconfirmed-text-navigation-with-page-cache-expected.txt: Added.
* platform/mac/editing/input/unconfirmed-text-navigation-with-page-cache.html: Added.
2013-05-17 Ryosuke Niwa <rniwa@webkit.org>
Mac rebaselines; also remove some entries.
......
......@@ -319,6 +319,9 @@ sputnik/Unicode/Unicode_218/S7.6_A5.2_T1.html
# Unclear yet why this test fails.
editing/secure-input/password-input-focusing-to-different-frame.html [ Failure ]
# textInputController.hasMarkedText is implemented, but gives a wrong result for some reason.
platform/mac/editing/input/unconfirmed-text-navigation-with-page-cache.html
### END OF (3) Unclassified failures
########################################
......
<html>
<body>
<div contenteditable></div>
<script>
// Notify opener.
window.onpageshow = function() { opener.postMessage('first-page', '*'); }
// Our opener will tell us to perform various loads.
window.addEventListener('message', function(event) {
// Navigate to other page.
if (event.data === 'navigate-other-page') {
window.location = 'other-page.html';
return;
}
if (event.data === 'add-unconfirmed-text') {
textInputController.setMarkedText("test", 0, 4);
return;
}
if (event.data === 'has-marked-text') {
var result;
// DumpRenderTree does not restore textInputController after navigating back.
// FIXME: Fix textInputController to work in restored pages.
try {
// Workaround for a DRT vs. WTR difference - one returns a number, and another a boolean.
// FIXME: Fix the tools instead of working around the difference.
result = textInputController.hasMarkedText() ? "1" : "0";
} catch (ex) {
result = ex;
}
opener.postMessage('has-marked-text: ' + result, '*');
return;
}
}, false);
var input = document.getElementsByTagName("div")[0];
input.focus();
</script>
</body>
</html>
<html>
<body>
<script>
// Notify opener.
window.onpageshow = function() { opener.postMessage('other-page', '*'); }
// Our opener will tell us to perform various loads.
window.addEventListener('message', function(event) {
// Navigate first resource.
if (event.data === 'navigate-first-page') {
window.location = 'first-page.html';
return;
}
// Navigate back.
if (event.data === 'navigate-back') {
window.history.back();
return;
}
}, false);
</script>
</body>
</html>
Adding unconfirmed inline text...
PASS event.data is 'has-marked-text: 1'
Navigating away from a page with unconfirmed inline input...
Navigating back to the first page...
FAIL event.data should be has-marked-text: 0. Was has-marked-text: ReferenceError: Trying to access object from destroyed plug-in..
PASS successfullyParsed is true
TEST COMPLETE
<html>
<head>
<title>Test what happens when navigating from a page with unconfirmed inline input when the page is cacheable</title>
<script src="../../../../fast/js/resources/js-test-pre.js"></script>
</head>
<body>
<script>
jsTestIsAsync = true;
if (window.testRunner) {
testRunner.setCanOpenWindows();
testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
}
// Window we will be controlling.
var target;
// Counter for visits to first page.
var firstPageVisits = 0;
window.addEventListener('message', function(event) {
if (event.data === 'first-page') {
firstPageVisits++;
if (firstPageVisits == 1) {
debug("Adding unconfirmed inline text...");
target.postMessage('add-unconfirmed-text', '*');
target.postMessage('has-marked-text', '*');
} else {
target.postMessage('has-marked-text', '*');
}
return;
}
if (event.data.indexOf('has-marked-text: ') == 0) {
if (firstPageVisits == 1) {
shouldBe("event.data", "'has-marked-text: 1'");
debug("Navigating away from a page with unconfirmed inline input...");
target.postMessage('navigate-other-page', '*');
} else {
shouldBe("event.data", "'has-marked-text: 0'");
finishJSTest();
}
return;
}
if (event.data === 'other-page') {
debug("Navigating back to the first page...");
target.postMessage('navigate-back', '*');
return;
}
}, false);
// Open the target window and we will start to exchange messages.
onload = function() { target = window.open('resources/first-page.html'); }
</script>
<script src="../../../../fast/js/resources/js-test-post.js"></script>
</body>
</html>
2013-05-17 Alexey Proskuryakov <ap@apple.com>
Text input is largely broken when there are subframes loading
http://bugs.webkit.org/show_bug.cgi?id=59121
<rdar://problem/9320468>
Reviewed by Darin Adler.
This addresses text input being abandoned when another frame in a page is navigated.
There are still many opportunities for improvement:
- Track other cases where WebCore interferes may want to cancel input without
direct user action (e.g. deleting the whole editable element on a timer).
- Fix how dictionary panel and autocorrection are dismissed (they still have the
same issue, and get dismissed with any frame navigation).
Test: platform/mac/editing/input/unconfirmed-text-navigation-with-page-cache.html
* loader/FrameLoader.h:
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::willTransitionToCommitted): Make sure that we
do not keep an inline session in a frame that's no longer active, as WebKit2 no
longer takes care of this case (and more of the logic should be in WebCore anyway).
(WebCore::FrameLoader::commitProvisionalLoad): Added a hook that gets invoked right
before transitioning to committed state starts. We may want to move more code here
eventually, e.g. from Frame::setView.
2013-05-17 Christophe Dumez <ch.dumez@sisa.samsung.com>
Get rid of [CustomGetter] for global named constructors
......
......@@ -474,6 +474,18 @@ void FrameLoader::stop()
icon()->stopLoader();
}
void FrameLoader::willTransitionToCommitted()
{
// This function is called when a frame is still fully in place (not cached, not detached), but will be replaced.
if (m_frame->editor()->hasComposition()) {
// The text was already present in DOM, so it's better to confirm than to cancel the composition.
m_frame->editor()->confirmComposition();
if (EditorClient* editorClient = m_frame->editor()->client())
editorClient->respondToChangedSelection(m_frame);
}
}
bool FrameLoader::closeURL()
{
history()->saveDocumentState();
......@@ -1681,6 +1693,8 @@ void FrameLoader::commitProvisionalLoad()
m_frame->document() ? m_frame->document()->url().elidedString().utf8().data() : "",
pdl ? pdl->url().elidedString().utf8().data() : "<no provisional DocumentLoader>");
willTransitionToCommitted();
// Check to see if we need to cache the page we are navigating away from into the back/forward cache.
// We are doing this here because we know for sure that a new page is about to be loaded.
HistoryItem* item = history()->currentItem();
......
......@@ -367,6 +367,7 @@ private:
void prepareForLoadStart();
void provisionalLoadStarted();
void willTransitionToCommitted();
bool didOpenURL();
void scheduleCheckCompleted();
......
2013-05-16 Alexey Proskuryakov <ap@apple.com>
Text input is largely broken when there are subframes loading
http://bugs.webkit.org/show_bug.cgi?id=59121
<rdar://problem/9320468>
Reviewed by Darin Adler.
* UIProcess/PageClient.h:
* UIProcess/API/mac/PageClientImpl.h:
* UIProcess/API/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::updateSecureInputState): Separated secure input state
updating into a separate function. Removed updateTextInputState, we don't need
to go through PageClient to implement its behavior at all.
(WebKit::PageClientImpl::dismissDictionaryLookupPanel): Added a FIXME.
* UIProcess/API/mac/WKView.mm:
* UIProcess/API/mac/WKViewInternal.h:
Removed _updateTextInputStateIncludingSecureInputState.
* UIProcess/WebPageProxy.h: Added m_temporarilyClosedComposition, which helps
to figure out that WebCore decided to close a composition. The issue is that WebCore
would first send an EditorState with hasComposition set to false, and with
shouldIgnoreCompositionSelectionChange set to true, at which time we forget the
previous m_editorState, but can't make any decisions based on this transient state.
We should find a way to simplify this (maybe not send these updates with
shouldIgnoreCompositionSelectionChange at all?)
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::WebPageProxy): Initialize m_temporarilyClosedComposition.
(WebKit::WebPageProxy::didCommitLoadForFrame): Removed the code to kill a composition
when any frame commits a load, which made no sense (along with surrounding code,
which will unfortunately survive longer).
(WebKit::WebPageProxy::editorStateChanged): Implemented state updating here,
we don't need to go to WKView.mm to implement this logic. Figure out when WebCore
discards a composition, and notify input methods about this.
(WebKit::WebPageProxy::resetStateAfterProcessExited): Reset m_temporarilyClosedComposition.
Added some FIXMEs.
2013-05-17 Manuel Rego Casasnovas <rego@igalia.com>
[WK2] Add support for selectTrailingWhitespaceEnabled setting
......
......@@ -82,7 +82,7 @@ private:
virtual void setDragImage(const WebCore::IntPoint& clientPosition, PassRefPtr<ShareableBitmap> dragImage, bool isLinkDrag);
virtual void setPromisedData(const String& pasteboardName, PassRefPtr<WebCore::SharedBuffer> imageBuffer, const String& filename, const String& extension, const String& title,
const String& url, const String& visibleUrl, PassRefPtr<WebCore::SharedBuffer> archiveBuffer);
virtual void updateTextInputState(bool updateSecureInputState);
virtual void updateSecureInputState() OVERRIDE;
virtual void resetSecureInputState() OVERRIDE;
virtual void notifyInputContextAboutDiscardedComposition() OVERRIDE;
......
......@@ -326,9 +326,9 @@ void PageClientImpl::setPromisedData(const String& pasteboardName, PassRefPtr<Sh
[m_wkView _setPromisedData:image.get() withFileName:filename withExtension:extension withTitle:title withURL:url withVisibleURL:visibleUrl withArchive:archiveBuffer.get() forPasteboard:pasteboardName];
}
void PageClientImpl::updateTextInputState(bool updateSecureInputState)
void PageClientImpl::updateSecureInputState()
{
[m_wkView _updateTextInputStateIncludingSecureInputState:updateSecureInputState];
[m_wkView _updateSecureInputState];
}
void PageClientImpl::resetSecureInputState()
......@@ -502,6 +502,7 @@ void PageClientImpl::didPerformDictionaryLookup(const AttributedString& text, co
void PageClientImpl::dismissDictionaryLookupPanel()
{
// FIXME: We don't know which panel we are dismissing, it may not even be in the current page (see <rdar://problem/13875766>).
WKHideWordDefinitionWindow();
}
......
......@@ -2900,23 +2900,6 @@ static NSString *pathWithUniqueFilenameForPath(NSString *path)
_data->_inSecureInputState = isInPasswordField;
}
- (void)_updateTextInputStateIncludingSecureInputState:(BOOL)updateSecureInputState
{
const EditorState& editorState = _data->_page->editorState();
if (updateSecureInputState) {
// This is a temporary state when editing. Flipping secure input state too quickly can expose race conditions.
if (!editorState.selectionIsNone)
[self _updateSecureInputState];
}
if (!editorState.hasComposition || editorState.shouldIgnoreCompositionSelectionChange)
return;
_data->_page->cancelComposition();
[self _notifyInputContextAboutDiscardedComposition];
}
- (void)_resetSecureInputState
{
if (_data->_inSecureInputState) {
......
......@@ -84,7 +84,6 @@ namespace WebKit {
- (void)_updateSecureInputState;
- (void)_resetSecureInputState;
- (void)_notifyInputContextAboutDiscardedComposition;
- (void)_updateTextInputStateIncludingSecureInputState:(BOOL)updateSecureInputState;
- (WebKit::ColorSpaceData)_colorSpace;
......
......@@ -156,7 +156,7 @@ public:
virtual bool interpretKeyEvent(const NativeWebKeyboardEvent&, Vector<WebCore::KeypressCommand>&) = 0;
virtual bool executeSavedCommandBySelector(const String& selector) = 0;
virtual void setDragImage(const WebCore::IntPoint& clientPosition, PassRefPtr<ShareableBitmap> dragImage, bool isLinkDrag) = 0;
virtual void updateTextInputState(bool updateSecureInputState) = 0;
virtual void updateSecureInputState() = 0;
virtual void resetSecureInputState() = 0;
virtual void notifyInputContextAboutDiscardedComposition() = 0;
virtual void makeFirstResponder() = 0;
......
......@@ -245,6 +245,7 @@ WebPageProxy::WebPageProxy(PageClient* pageClient, PassRefPtr<WebProcessProxy> p
, m_isVisible(m_pageClient->isViewVisible())
, m_backForwardList(WebBackForwardList::create(this))
, m_loadStateAtProcessExit(WebFrameProxy::LoadStateFinished)
, m_temporarilyClosedComposition(false)
, m_textZoomFactor(1)
, m_pageZoomFactor(1)
, m_pageScaleFactor(1)
......@@ -2260,8 +2261,7 @@ void WebPageProxy::didCommitLoadForFrame(uint64_t frameID, const String& mimeTyp
#if PLATFORM(MAC)
// FIXME (bug 59111): didCommitLoadForFrame comes too late when restoring a page from b/f cache, making us disable secure event mode in password fields.
// FIXME (bug 59121): A load going on in one frame shouldn't affect typing in sibling frames.
m_pageClient->notifyInputContextAboutDiscardedComposition();
// FIXME: A load going on in one frame shouldn't affect text editing in other frames on the page.
m_pageClient->resetSecureInputState();
dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
m_pageClient->dismissDictionaryLookupPanel();
......@@ -3069,12 +3069,28 @@ void WebPageProxy::editorStateChanged(const EditorState& editorState)
{
#if PLATFORM(MAC)
bool couldChangeSecureInputState = m_editorState.isInPasswordField != editorState.isInPasswordField || m_editorState.selectionIsNone;
bool closedComposition = !editorState.shouldIgnoreCompositionSelectionChange && !editorState.hasComposition && (m_editorState.hasComposition || m_temporarilyClosedComposition);
m_temporarilyClosedComposition = editorState.shouldIgnoreCompositionSelectionChange && (m_temporarilyClosedComposition || m_editorState.hasComposition) && !editorState.hasComposition;
#endif
m_editorState = editorState;
#if PLATFORM(MAC)
m_pageClient->updateTextInputState(couldChangeSecureInputState);
// Selection being none is a temporary state when editing. Flipping secure input state too quickly was causing trouble (not fully understood).
if (couldChangeSecureInputState && !editorState.selectionIsNone)
m_pageClient->updateSecureInputState();
if (editorState.shouldIgnoreCompositionSelectionChange)
return;
if (closedComposition)
m_pageClient->notifyInputContextAboutDiscardedComposition();
if (editorState.hasComposition) {
// Abandon the current inline input session if selection changed for any other reason but an input method changing the composition.
// FIXME: This logic should be in WebCore, no need to round-trip to UI process to cancel the composition.
cancelComposition();
m_pageClient->notifyInputContextAboutDiscardedComposition();
}
#elif PLATFORM(QT) || PLATFORM(EFL) || PLATFORM(GTK)
m_pageClient->updateTextInputState();
#endif
......@@ -3916,6 +3932,10 @@ void WebPageProxy::resetStateAfterProcessExited()
m_touchEventQueue.clear();
#endif
// FIXME: Reset m_editorState.
// FIXME: Notify input methods about abandoned composition.
m_temporarilyClosedComposition = false;
#if PLATFORM(MAC)
dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
m_pageClient->dismissDictionaryLookupPanel();
......
......@@ -1131,6 +1131,7 @@ private:
WebFrameProxy::LoadState m_loadStateAtProcessExit;
EditorState m_editorState;
bool m_temporarilyClosedComposition; // Editor state changed from hasComposition to !hasComposition, but that was only with shouldIgnoreCompositionSelectionChange yet.
double m_textZoomFactor;
double m_pageZoomFactor;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment