Bug 157995 – [iOS] Allow clients to override the type of an input field

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chelsea Pugh]

Allow clients to set the type of an input field. Patch forthcoming.

[Chelsea Pugh]

Created attachment 279580 [details]
[iOS] Allow clients to set the type of an input field.txt

Patch for this bug

Comment on attachment 279580 [details]
[iOS] Allow clients to set the type of an input field.txt

View in context: https://bugs.webkit.org/attachment.cgi?id=279580&action=review

> Source/WebKit2/ChangeLog:3
> +        [iOS] Allow clients to set the type of an input field

Perhaps “override” instead of “set”?

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:366
> +    [_contentView reloadInputViews];

It’s unfortunate that if the client sets the text content type and then immediately sets the input view, we’ll reload input views twice. Might be good to follow up with a change that coalesces these if possible.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3032
> +    if (NSString *textContentType = [_formInputSession textContentType])
> +        [_traits setTextContentType:textContentType];

A little strange to let the switch statement above set the type only to have it set to something else here.

[Chelsea Pugh]

Created attachment 279586 [details]
Patch for [iOS] Allow clients to set the type of an input field v2

Patch version 2

[WebKit Commit Bot]

Attachment 279586 [details] did not pass style-queue:


ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3001:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3002:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3004:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3005:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3007:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3008:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3010:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3011:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3013:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3014:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3016:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 11 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chelsea Pugh]

Created attachment 279592 [details]
v2 with better spacing

[Chelsea Pugh]

Created attachment 279594 [details]
Fix ChangeLog spacing

Comment on attachment 279594 [details]
Fix ChangeLog spacing

View in context: https://bugs.webkit.org/attachment.cgi?id=279594&action=review

> Source/WebKit2/UIProcess/API/Cocoa/_WKFormInputSession.h:46
> +@property (nonatomic, strong) NSString *textContentType WK_AVAILABLE(NA, WK_IOS_TBA);

This should be copy, not strong…

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:362
> +    if (textContentType == _textContentType)

…so this should check for equality in addition to checking for identity…

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:365
> +    _textContentType = textContentType;

…and this should make a copy and adopt it.

> Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3021
> +    if (NSString *textContentType = [_formInputSession textContentType])
> +        [_traits setTextContentType:textContentType];
> +    else if (NSString *textContentType = contentTypeFromFieldName(_assistedNodeInformation.autofillFieldName))
> +        [_traits setTextContentType:textContentType];

The problem with this implementation is that if the field’s autofillFieldName is, say, None, and the client uses the SPI to set textContentType to some non-nil value and then back to nil, the traits’ textContentType doesn’t revert to whatever the default value is for None. Is there a way to avoid this issue?

(In reply to comment #7)

> > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3021
> > +    if (NSString *textContentType = [_formInputSession textContentType])
> > +        [_traits setTextContentType:textContentType];
> > +    else if (NSString *textContentType = contentTypeFromFieldName(_assistedNodeInformation.autofillFieldName))
> > +        [_traits setTextContentType:textContentType];
> 
> The problem with this implementation is that if the field’s
> autofillFieldName is, say, None, and the client uses the SPI to set
> textContentType to some non-nil value and then back to nil, the traits’
> textContentType doesn’t revert to whatever the default value is for None. Is
> there a way to avoid this issue?

I suppose this may already be a problem with the existing code when focus moves from a filed with an autofillFieldName that has a corresponding textContentType to one with an autofillFieldName that doesn’t.

[Chelsea Pugh]

(In reply to comment #8)
> (In reply to comment #7)
> 
> > > Source/WebKit2/UIProcess/ios/WKContentViewInteraction.mm:3021
> > > +    if (NSString *textContentType = [_formInputSession textContentType])
> > > +        [_traits setTextContentType:textContentType];
> > > +    else if (NSString *textContentType = contentTypeFromFieldName(_assistedNodeInformation.autofillFieldName))
> > > +        [_traits setTextContentType:textContentType];
> > 
> > The problem with this implementation is that if the field’s
> > autofillFieldName is, say, None, and the client uses the SPI to set
> > textContentType to some non-nil value and then back to nil, the traits’
> > textContentType doesn’t revert to whatever the default value is for None. Is
> > there a way to avoid this issue?

I guess we could have an else that sets the text content type to nil (its default value). This way, if neither the input session or the autofillFieldName has something to get the textContentType from, we set it back to its original value.

> 
> I suppose this may already be a problem with the existing code when focus
> moves from a filed with an autofillFieldName that has a corresponding
> textContentType to one with an autofillFieldName that doesn’t.

I believe when we move to a new field, new a new UITextInputTraits object is created, so the textContentType would start back at nil (its default value).

[Chelsea Pugh]

Created attachment 279676 [details]
[iOS] Allow clients to set the type of an input field v3

[Chelsea Pugh]

Created attachment 279689 [details]
[iOS] Allow clients to set the type of an input field v4

Fix build.

[WebKit Commit Bot]

Comment on attachment 279689 [details]
[iOS] Allow clients to set the type of an input field v4

Clearing flags on attachment: 279689

Committed r201347: <http://trac.webkit.org/changeset/201347>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.