Skip to content

Commit cd8a0b1

Browse files
[iOS] Tweak support for classifying form controls (followup to r222487)
https://bugs.webkit.org/show_bug.cgi?id=177917 <rdar://problem/34820122> Reviewed by Dean Jackson. Source/WebKit: This patch follows up with <http://trac.webkit.org/r222487>. It combines the functionality of two SPI hooks implemented on WKContentView into a single method that vends a context dictionary, and additionally addresses an issue with the original implementation, wherein some cached state on WebPageProxy is set upon starting node assistance, but is never unset when stopping node assistance, nor set anywhere else. See per-method comments for more detail. * UIProcess/WebPageProxy.h: Remove members m_acceptsAutofilledLoginCredentials and m_representingPageURL from WebPageProxy. This state is retrieved from the AssistedNodeInformation struct when starting node assistance, but is never reset anywhere else. Instead of introducing new members to remember this state, we can just use the WKContentView's current assisted node information. This also means that programmatically focusing forms (without user gesture) will no longer cause WKContentView to accept autofilled login credentials, since we bail out of node assistance and don't begin an input session. * UIProcess/ios/WKContentView.mm: (-[WKContentView acceptsAutofilledLoginCredentials]): Deleted. (-[WKContentView representingPageURL]): Deleted. * UIProcess/ios/WKContentViewInteraction.mm: (-[WKContentView _autofillContext]): Merge functionality of the previous two SPI hooks, such that -_autofillContext will return a non-null dictionary containing the URL of the focused element's document if and only if WKContentView accepts autofilled login credentials, and there exists a representing page URL. When the page stops assisting the focused node, we set the AssistedNodeInformation's element type to None, so we additionally bail and return nil if the element type is None. As an aside, it seems a more reasonable approach to resetting state upon stopping node assistance is to just completely reset _assistedNodeInformation to its initial value, i.e. via _assistedNodeInformation = { }. It's not clear whether there are behaviors relying on the fact that all members but the element type in the content view's assisted node information could be stale, so this seems worthy of some investigation. * UIProcess/ios/WebPageProxyIOS.mm: (WebKit::WebPageProxy::startAssistingNode): (WebKit::WebPageProxy::acceptsAutofilledLoginCredentials): Deleted. (WebKit::WebPageProxy::representingPageURL): Deleted. Source/WebKitLegacy/mac: Implement _autofillContext in legacy WebKit, and remove the two previous SPI hooks. * DOM/DOMHTMLInputElement.mm: (-[DOMHTMLInputElement _autofillContext]): (-[DOMHTMLInputElement acceptsAutofilledLoginCredentials]): Deleted. (-[DOMHTMLInputElement representingPageURL]): Deleted. Tools: Minor cleanup around autofill API tests added in r222487. Additionally, augments some of these API tests to verify that after blurring the focused element, the content view no longer accepts autofill credentials (see WebKit ChangeLog for more details). Also augments tests to verify that the URL in the autofill context matches the document URL. * TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm: Remove the USE(APPLE_INTERNAL_SDK) guard for these API tests. (newUIKeyboardLoginCredentialsSuggestion): Add a stub implementation of UIKeyboardLoginCredentialsSuggestion. This allows [UIKeyboardLoginCredentialsSuggestion new] to return a nonnull object, which allows these API tests to verify that the credential filling codepath works as intended without any additional UIKit changes. Currently, tests for the value of username and password fields are passing even though the fields are not being populated because the expected string values are null, and the observed value is an empty string. We instead check the literal string values here instead of credentialSuggestion's properties, so that tests verifying the behavior of -insertTextSuggestion: will require username and password inputs to be populated. (-[TestInputDelegate _webView:focusShouldStartInputSession:]): (-[AutofillTestView initWithFrame:]): (-[AutofillTestView _autofillInputView]): (-[AutofillTestView textInputHasAutofillContext]): (TestWebKitAPI::TEST): Add an additional API test to verify that programmatic focus without user interaction (and also without testing overrides) does not activate autofill. (createUIKeyboardLoginCredentialsSuggestion): Deleted. (-[WKWebView _privateTextInput]): Deleted. * TestWebKitAPI/ios/UIKitSPI.h: Minor gardening to remove iOS version >= 11 guards that are now unnecessary. Additionally, add some more private header imports (with corresponding interface definitions for building and running with the public SDK). git-svn-id: http://svn.webkit.org/repository/webkit/trunk@222991 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent eaab522 commit cd8a0b1

File tree

10 files changed

+255
-118
lines changed

10 files changed

+255
-118
lines changed

Source/WebKit/ChangeLog

+46
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,49 @@
1+
2017-10-06 Wenson Hsieh <[email protected]>
2+
3+
[iOS] Tweak support for classifying form controls (followup to r222487)
4+
https://bugs.webkit.org/show_bug.cgi?id=177917
5+
<rdar://problem/34820122>
6+
7+
Reviewed by Dean Jackson.
8+
9+
This patch follows up with <http://trac.webkit.org/r222487>. It combines the functionality of two SPI hooks
10+
implemented on WKContentView into a single method that vends a context dictionary, and additionally addresses an
11+
issue with the original implementation, wherein some cached state on WebPageProxy is set upon starting node
12+
assistance, but is never unset when stopping node assistance, nor set anywhere else. See per-method comments for
13+
more detail.
14+
15+
* UIProcess/WebPageProxy.h:
16+
17+
Remove members m_acceptsAutofilledLoginCredentials and m_representingPageURL from WebPageProxy. This state is
18+
retrieved from the AssistedNodeInformation struct when starting node assistance, but is never reset anywhere
19+
else. Instead of introducing new members to remember this state, we can just use the WKContentView's current
20+
assisted node information.
21+
22+
This also means that programmatically focusing forms (without user gesture) will no longer cause WKContentView
23+
to accept autofilled login credentials, since we bail out of node assistance and don't begin an input session.
24+
25+
* UIProcess/ios/WKContentView.mm:
26+
(-[WKContentView acceptsAutofilledLoginCredentials]): Deleted.
27+
(-[WKContentView representingPageURL]): Deleted.
28+
* UIProcess/ios/WKContentViewInteraction.mm:
29+
(-[WKContentView _autofillContext]):
30+
31+
Merge functionality of the previous two SPI hooks, such that -_autofillContext will return a non-null dictionary
32+
containing the URL of the focused element's document if and only if WKContentView accepts autofilled login
33+
credentials, and there exists a representing page URL.
34+
35+
When the page stops assisting the focused node, we set the AssistedNodeInformation's element type to None, so we
36+
additionally bail and return nil if the element type is None. As an aside, it seems a more reasonable approach to
37+
resetting state upon stopping node assistance is to just completely reset _assistedNodeInformation to its initial
38+
value, i.e. via _assistedNodeInformation = { }. It's not clear whether there are behaviors relying on the fact
39+
that all members but the element type in the content view's assisted node information could be stale, so this
40+
seems worthy of some investigation.
41+
42+
* UIProcess/ios/WebPageProxyIOS.mm:
43+
(WebKit::WebPageProxy::startAssistingNode):
44+
(WebKit::WebPageProxy::acceptsAutofilledLoginCredentials): Deleted.
45+
(WebKit::WebPageProxy::representingPageURL): Deleted.
46+
147
2017-10-06 Yousuke Kimoto <[email protected]>
248

349
[WinCairo] Add shared curl files

Source/WebKit/UIProcess/WebPageProxy.h

-4
Original file line numberDiff line numberDiff line change
@@ -559,8 +559,6 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
559559
void setIsScrollingOrZooming(bool);
560560
void requestRectsForGranularityWithSelectionOffset(WebCore::TextGranularity, uint32_t offset, WTF::Function<void(const Vector<WebCore::SelectionRect>&, CallbackBase::Error)>&&);
561561
void requestRectsAtSelectionOffsetWithText(int32_t offset, const String&, WTF::Function<void(const Vector<WebCore::SelectionRect>&, CallbackBase::Error)>&&);
562-
bool acceptsAutofilledLoginCredentials();
563-
WebCore::URL representingPageURL();
564562
void autofillLoginCredentials(const String& username, const String& password);
565563
#if ENABLE(DATA_INTERACTION)
566564
void didPerformDataInteractionControllerOperation(bool handled);
@@ -1998,8 +1996,6 @@ class WebPageProxy : public API::ObjectImpl<API::Object::Type::Page>
19981996
bool m_hasDeferredStartAssistingNode { false };
19991997
std::unique_ptr<NodeAssistanceArguments> m_deferredNodeAssistanceArguments;
20001998
bool m_forceAlwaysUserScalable { false };
2001-
bool m_acceptsAutofilledLoginCredentials { false };
2002-
WebCore::URL m_representingPageURL;
20031999
#endif
20042000

20052001
#if ENABLE(POINTER_LOCK)

Source/WebKit/UIProcess/ios/WKContentView.mm

-10
Original file line numberDiff line numberDiff line change
@@ -257,16 +257,6 @@ - (WebPageProxy*)page
257257
return _page.get();
258258
}
259259

260-
- (BOOL)acceptsAutofilledLoginCredentials
261-
{
262-
return _page->acceptsAutofilledLoginCredentials();
263-
}
264-
265-
- (NSURL *)representingPageURL
266-
{
267-
return _page->representingPageURL();
268-
}
269-
270260
- (void)willMoveToWindow:(UIWindow *)newWindow
271261
{
272262
NSNotificationCenter *defaultCenter = [NSNotificationCenter defaultCenter];

Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

+12
Original file line numberDiff line numberDiff line change
@@ -4518,6 +4518,18 @@ - (void)_restoreCalloutBarIfNeeded
45184518
return dragItems;
45194519
}
45204520

4521+
- (NSDictionary *)_autofillContext
4522+
{
4523+
if (_assistedNodeInformation.elementType == InputType::None || !_assistedNodeInformation.acceptsAutofilledLoginCredentials)
4524+
return nil;
4525+
4526+
NSURL *platformURL = _assistedNodeInformation.representingPageURL;
4527+
if (!platformURL)
4528+
return nil;
4529+
4530+
return @{ @"_WebViewURL" : platformURL };
4531+
}
4532+
45214533
#pragma mark - UIDragInteractionDelegate
45224534

45234535
- (BOOL)_dragInteraction:(UIDragInteraction *)interaction shouldDelayCompetingGestureRecognizer:(UIGestureRecognizer *)competingGestureRecognizer

Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm

-12
Original file line numberDiff line numberDiff line change
@@ -891,8 +891,6 @@ static bool exceedsRenderTreeSizeSizeThreshold(uint64_t thresholdSize, uint64_t
891891

892892
void WebPageProxy::startAssistingNode(const AssistedNodeInformation& information, bool userIsInteracting, bool blurPreviousNode, const UserData& userData)
893893
{
894-
m_acceptsAutofilledLoginCredentials = information.acceptsAutofilledLoginCredentials;
895-
m_representingPageURL = information.representingPageURL;
896894
API::Object* userDataObject = process().transformHandlesToObjects(userData.object()).get();
897895
if (m_editorState.isMissingPostLayoutData) {
898896
m_deferredNodeAssistanceArguments = std::make_unique<NodeAssistanceArguments>(NodeAssistanceArguments { information, userIsInteracting, blurPreviousNode, userDataObject });
@@ -912,16 +910,6 @@ static bool exceedsRenderTreeSizeSizeThreshold(uint64_t thresholdSize, uint64_t
912910
m_pageClient.stopAssistingNode();
913911
}
914912

915-
bool WebPageProxy::acceptsAutofilledLoginCredentials()
916-
{
917-
return m_acceptsAutofilledLoginCredentials;
918-
}
919-
920-
WebCore::URL WebPageProxy::representingPageURL()
921-
{
922-
return m_representingPageURL;
923-
}
924-
925913
void WebPageProxy::autofillLoginCredentials(const String& username, const String& password)
926914
{
927915
m_process->send(Messages::WebPage::AutofillLoginCredentials(username, password), m_pageID);

Source/WebKitLegacy/mac/ChangeLog

+15
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
2017-10-06 Wenson Hsieh <[email protected]>
2+
3+
[iOS] Tweak support for classifying form controls (followup to r222487)
4+
https://bugs.webkit.org/show_bug.cgi?id=177917
5+
<rdar://problem/34820122>
6+
7+
Reviewed by Dean Jackson.
8+
9+
Implement _autofillContext in legacy WebKit, and remove the two previous SPI hooks.
10+
11+
* DOM/DOMHTMLInputElement.mm:
12+
(-[DOMHTMLInputElement _autofillContext]):
13+
(-[DOMHTMLInputElement acceptsAutofilledLoginCredentials]): Deleted.
14+
(-[DOMHTMLInputElement representingPageURL]): Deleted.
15+
116
2017-10-05 Keith Miller <[email protected]>
217

318
Unreviewed, tapi builds without optimization so we should have TAPI passes -DRELEASE_WITHOUT_OPTIMIZATIONS.

Source/WebKitLegacy/mac/DOM/DOMHTMLInputElement.mm

+9-8
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
#import "DOMPrivate.h"
3535
#import "ExceptionHandlers.h"
3636

37-
// FIXME <radar:34583628>: Simplyfy this once the UIKit work is available in the build.
37+
// FIXME: Simplify this once <rdar://problem/34583628> is available in the build.
3838
#if USE(APPLE_INTERNAL_SDK) && TARGET_OS_IPHONE
3939
#if __has_include(<UIKit/UIKeyboardLoginCredentialsSuggestion.h>)
4040
#import <UIKit/UIKeyboardLoginCredentialsSuggestion.h>
@@ -683,16 +683,17 @@ - (void)setValueForUser:(NSString *)inValue
683683
IMPL->setValueForUser(inValue);
684684
}
685685

686-
- (BOOL)acceptsAutofilledLoginCredentials
686+
- (NSDictionary *)_autofillContext
687687
{
688688
WebCore::JSMainThreadNullState state;
689-
return !!WebCore::AutofillElements::computeAutofillElements(*IMPL);
690-
}
689+
if (!WebCore::AutofillElements::computeAutofillElements(*IMPL))
690+
return nil;
691691

692-
- (NSURL *)representingPageURL
693-
{
694-
WebCore::JSMainThreadNullState state;
695-
return [NSURL URLWithString:self.ownerDocument.URL];
692+
NSURL *documentURL = [NSURL URLWithString:self.ownerDocument.URL];
693+
if (!documentURL)
694+
return nil;
695+
696+
return @{ @"_WebViewURL" : documentURL };
696697
}
697698

698699
#if USE(APPLE_INTERNAL_SDK) && TARGET_OS_IPHONE

Tools/ChangeLog

+43
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,46 @@
1+
2017-10-06 Wenson Hsieh <[email protected]>
2+
3+
[iOS] Tweak support for classifying form controls (followup to r222487)
4+
https://bugs.webkit.org/show_bug.cgi?id=177917
5+
<rdar://problem/34820122>
6+
7+
Reviewed by Dean Jackson.
8+
9+
Minor cleanup around autofill API tests added in r222487. Additionally, augments some of these API tests to
10+
verify that after blurring the focused element, the content view no longer accepts autofill credentials (see
11+
WebKit ChangeLog for more details). Also augments tests to verify that the URL in the autofill context matches
12+
the document URL.
13+
14+
* TestWebKitAPI/Tests/ios/WKWebViewAutofillTests.mm:
15+
16+
Remove the USE(APPLE_INTERNAL_SDK) guard for these API tests.
17+
18+
(newUIKeyboardLoginCredentialsSuggestion):
19+
20+
Add a stub implementation of UIKeyboardLoginCredentialsSuggestion. This allows
21+
[UIKeyboardLoginCredentialsSuggestion new] to return a nonnull object, which allows these API tests to verify
22+
that the credential filling codepath works as intended without any additional UIKit changes. Currently, tests
23+
for the value of username and password fields are passing even though the fields are not being populated because
24+
the expected string values are null, and the observed value is an empty string. We instead check the literal
25+
string values here instead of credentialSuggestion's properties, so that tests verifying the behavior of
26+
-insertTextSuggestion: will require username and password inputs to be populated.
27+
28+
(-[TestInputDelegate _webView:focusShouldStartInputSession:]):
29+
(-[AutofillTestView initWithFrame:]):
30+
(-[AutofillTestView _autofillInputView]):
31+
(-[AutofillTestView textInputHasAutofillContext]):
32+
(TestWebKitAPI::TEST):
33+
34+
Add an additional API test to verify that programmatic focus without user interaction (and also without testing
35+
overrides) does not activate autofill.
36+
37+
(createUIKeyboardLoginCredentialsSuggestion): Deleted.
38+
(-[WKWebView _privateTextInput]): Deleted.
39+
* TestWebKitAPI/ios/UIKitSPI.h:
40+
41+
Minor gardening to remove iOS version >= 11 guards that are now unnecessary. Additionally, add some more private
42+
header imports (with corresponding interface definitions for building and running with the public SDK).
43+
144
2017-10-06 Antti Koivisto <[email protected]>
245

346
Minor WeakPtr improvements

0 commit comments

Comments
 (0)