[webkit-reviews] review granted: [Bug 196740] [iOS] Input field on ddg.gg is auto focused when url is entered with the software keyboard : [Attachment 367142] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 22 16:43:33 PDT 2019

Megan Gardner <megan_gardner at apple.com> has granted Per Arne Vollan
<pvollan at apple.com>'s request for review:
Bug 196740: [iOS] Input field on ddg.gg is auto focused when url is entered
with the software keyboard

Attachment 367142: Patch


--- Comment #9 from Megan Gardner <megan_gardner at apple.com> ---
Comment on attachment 367142
  --> https://bugs.webkit.org/attachment.cgi?id=367142

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

So, I added chagingActivityState originally to make it so that the software
keyboard would not be dismissed if you backgrounded Safari, and then brought it
forward again. That is the activityState that was changing. I have noted that
this has problems when you background a page that has an auto-focused element
that has not brought up the keyboard because there has been no user
interaction. If you background that page, and bring it forward, the keyboard
shows up. Sending the whole bitfield across seems like a good way to make sure
that we're only showing the keyboard in the right cases, but at the risk of
bikesheding, if you're going to change the type of this flag, please make the
name more appropriate as well, Just 'activityState' should be good.

> Source/WebKit/UIProcess/WebPageProxy.h:348
>      RefPtr<API::Object> userData;

Please change to activityState, since this is no longer a bool.

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:4968
> +		   if (changingActivityState !=
WebCore::ActivityState::IsFocused && changingActivityState)

I think this would read better if the order was switched.

> Source/WebKit/WebProcess/WebPage/WebPage.h:1760

Init this with something appropriate if possible.

More information about the webkit-reviews mailing list