Bug 196018 - [iOS] Group UIWebFormAccessoryDelegate-related code and tighten it up a bit
Summary: [iOS] Group UIWebFormAccessoryDelegate-related code and tighten it up a bit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: iPhone / iPad iOS 12
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-20 11:23 PDT by Daniel Bates
Modified: 2019-03-20 12:00 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.55 KB, patch)
2019-03-20 11:39 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
To land (6.54 KB, patch)
2019-03-20 11:58 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2019-03-20 11:23:39 PDT
-[WKContentView _updateAccessory] is called from -hideKeyboard, which is invoked when we commit a load and don't have a form accessory view. No need to send nil messages as part of trying to update the non-existent form accessory view in this case. We should also group all UIWebFormAccessoryDelegate protocol methods for ease of discoverability and it's not possible to show and hide the AutoFill button from -_updateAccessory because we the show case needs a title based on the current design of the button added in bug #131343. So, remove a long standing FIXME.
Comment 1 Daniel Bates 2019-03-20 11:39:16 PDT
Created attachment 365368 [details]
Patch
Comment 2 Wenson Hsieh 2019-03-20 11:48:28 PDT
Comment on attachment 365368 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3753
> +    _page->setFocusedElementValue(String { });

Nit - wouldn't just { } be more concise?
Comment 3 Daniel Bates 2019-03-20 11:56:10 PDT
(In reply to Wenson Hsieh from comment #2)
> Comment on attachment 365368 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=365368&action=review
> 
> > Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:3753
> > +    _page->setFocusedElementValue(String { });
> 
> Nit - wouldn't just { } be more concise?

OK
Comment 4 Daniel Bates 2019-03-20 11:58:29 PDT
Created attachment 365377 [details]
To land
Comment 5 Daniel Bates 2019-03-20 11:59:13 PDT
Committed r243221: <https://trac.webkit.org/changeset/243221>
Comment 6 Radar WebKit Bug Importer 2019-03-20 12:00:21 PDT
<rdar://problem/49072320>