Bug 58686

Summary: Add takeFocus callback to WKPageUIClient
Product: WebKit Reporter: Jeff Miller <jeffm>
Component: WebKit2Assignee: Jeff Miller <jeffm>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dbates, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
Patch
none
Patch sam: review+

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