Bug 61153 - WebView loses firstResponder status when entering full-screen mode.
Summary: WebView loses firstResponder status when entering full-screen mode.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.6
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-05-19 16:10 PDT by Andy Estes
Modified: 2011-05-20 18:41 PDT (History)
1 user (show)

See Also:


Attachments
Patch (13.26 KB, patch)
2011-05-19 16:35 PDT, Andy Estes
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2011-05-19 16:10:08 PDT
Space button does not play/pause video on apple.com when in full-screen mode.
Comment 1 Andy Estes 2011-05-19 16:25:14 PDT
<rdar://problem/9400346>
Comment 2 Andy Estes 2011-05-19 16:35:16 PDT
Created attachment 94145 [details]
Patch
Comment 3 Darin Adler 2011-05-19 19:23:03 PDT
Comment on attachment 94145 [details]
Patch

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

> Source/WebKit/mac/Misc/WebNSWindowExtras.m:55
> +    if ([responder respondsToSelector:@selector(isDescendantOf:)]
> +        && [(id)responder isDescendantOf:view])

Should keep this expression all on one line to avoid awkward placement of the (id).

I suggest checking isKindOfClass:[NSView class] instead of specifically checking for this selector.

I don’t think it’s really so convenient to have this as an NSWindow method. I would suggest just making a helper function that does this. But I guess you made it a method so you could share with WebKit2. I am not really sure that’s the best practice way to make code you can share with WebKit1 and WebKit2. Might be worth asking Sam about that.
Comment 4 Andy Estes 2011-05-20 15:22:05 PDT
(In reply to comment #3)
> (From update of attachment 94145 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=94145&action=review
> 
> > Source/WebKit/mac/Misc/WebNSWindowExtras.m:55
> > +    if ([responder respondsToSelector:@selector(isDescendantOf:)]
> > +        && [(id)responder isDescendantOf:view])
> 
> Should keep this expression all on one line to avoid awkward placement of the (id).
> 
> I suggest checking isKindOfClass:[NSView class] instead of specifically checking for this selector.
> 
> I don’t think it’s really so convenient to have this as an NSWindow method. I would suggest just making a helper function that does this. But I guess you made it a method so you could share with WebKit2. I am not really sure that’s the best practice way to make code you can share with WebKit1 and WebKit2. Might be worth asking Sam about that.

Thanks Darin. I'll ping Sam for suggestions.
Comment 5 Andy Estes 2011-05-20 15:51:21 PDT
Committed r86995: <http://trac.webkit.org/changeset/86995>
Comment 6 Andy Estes 2011-05-20 18:41:46 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 94145 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=94145&action=review
> > 
> > > Source/WebKit/mac/Misc/WebNSWindowExtras.m:55
> > > +    if ([responder respondsToSelector:@selector(isDescendantOf:)]
> > > +        && [(id)responder isDescendantOf:view])
> > 
> > Should keep this expression all on one line to avoid awkward placement of the (id).
> > 
> > I suggest checking isKindOfClass:[NSView class] instead of specifically checking for this selector.
> > 
> > I don’t think it’s really so convenient to have this as an NSWindow method. I would suggest just making a helper function that does this. But I guess you made it a method so you could share with WebKit2. I am not really sure that’s the best practice way to make code you can share with WebKit1 and WebKit2. Might be worth asking Sam about that.
> 
> Thanks Darin. I'll ping Sam for suggestions.

Sam suggested I add the helper function to WebCore. I'll do that in a follow-up patch.