Bug 196018

Summary:

[iOS] Group UIWebFormAccessoryDelegate-related code and tighten it up a bit

Product:

WebKit

Reporter:

Daniel Bates <dbates>

Component:

WebKit Misc.

Assignee:

Daniel Bates <dbates>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

megan_gardner, thorton, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

iPhone / iPad

OS:

iOS 12

Attachments:

Description	Flags
Patch	none
To land	none

Daniel Bates

Reported 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.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Daniel Bates

Comment 1 2019-03-20 11:39:16 PDT

Created attachment 365368 [details] Patch

Wenson Hsieh

Comment 2 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?

Daniel Bates

Comment 3 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

Daniel Bates

Comment 4 2019-03-20 11:58:29 PDT

Created attachment 365377 [details] To land

Daniel Bates

Comment 5 2019-03-20 11:59:13 PDT

Committed r243221: <https://trac.webkit.org/changeset/243221>

Radar WebKit Bug Importer

Comment 6 2019-03-20 12:00:21 PDT

<rdar://problem/49072320>

Note You need to log in before you can comment on or make changes to this bug.