Bug 58686 - Add takeFocus callback to WKPageUIClient
Summary: Add takeFocus callback to WKPageUIClient
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Jeff Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-15 13:13 PDT by Jeff Miller
Modified: 2011-04-15 23:25 PDT (History)
4 users (show)

See Also:


Attachments
Patch (11.43 KB, patch)
2011-04-15 13:46 PDT, Jeff Miller
no flags Details | Formatted Diff | Diff
Patch (14.24 KB, patch)
2011-04-15 15:16 PDT, Jeff Miller
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Miller 2011-04-15 13:13:46 PDT
On Windows, we need to handle moving focus out of the web view in the client, so add a takeFocus callback to WKPageUIClient and stop handling taking focus in the framework in WKView.mm on the Mac.
Comment 1 Jeff Miller 2011-04-15 13:46:14 PDT
Created attachment 89845 [details]
Patch
Comment 2 Sam Weinig 2011-04-15 13:58:16 PDT
Comment on attachment 89845 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKPage.h:152
> +typedef void (*WKPageTakeFocusCallback)(WKPageRef page, bool direction, const void *clientInfo);

Instead of a bool, I think an enum would be better, e.g. WKFocusDirection.
Comment 3 Jeff Miller 2011-04-15 14:02:07 PDT
(In reply to comment #2)
> (From update of attachment 89845 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89845&action=review
> 
> > Source/WebKit2/UIProcess/API/C/WKPage.h:152
> > +typedef void (*WKPageTakeFocusCallback)(WKPageRef page, bool direction, const void *clientInfo);
> 
> Instead of a bool, I think an enum would be better, e.g. WKFocusDirection.

Sounds good, I'll upload another patch soon.
Comment 4 Jeff Miller 2011-04-15 15:16:25 PDT
Created attachment 89863 [details]
Patch
Comment 5 Sam Weinig 2011-04-15 16:00:15 PDT
Comment on attachment 89863 [details]
Patch

This will probably break Qt, you need to add the new client function to qwkpage.cpp.
Comment 6 Jeff Miller 2011-04-15 17:17:20 PDT
(In reply to comment #5)
> (From update of attachment 89863 [details])
> This will probably break Qt, you need to add the new client function to qwkpage.cpp.

Good catch, I will fix this before I commit.
Comment 7 Jeff Miller 2011-04-15 17:38:09 PDT
Committed r84064: <http://trac.webkit.org/changeset/84064>
Comment 8 WebKit Review Bot 2011-04-15 18:22:55 PDT
http://trac.webkit.org/changeset/84064 might have broken Leopard Intel Release (Build) and Leopard Intel Debug (Build)
Comment 9 Daniel Bates 2011-04-15 23:25:30 PDT
(In reply to comment #7)
> Committed r84064: <http://trac.webkit.org/changeset/84064>

This broke the Qt Linux Release build:
[[
...
obj/release/qwkpage.o:(.data.rel.ro._ZTV14QWKPagePrivate[vtable for QWKPagePrivate]+0xb0): undefined reference to `QWKPagePrivate::takeFocus(bool)'
collect2: ld returned 1 exit status
...
]]

Committed build fix in 84082 <http://trac.webkit.org/changeset/84082>.