Bug 61153

Summary: WebView loses firstResponder status when entering full-screen mode.
Product: WebKit Reporter: Andy Estes <aestes>
Component: DOMAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: sam
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.6   
Attachments:
Description Flags
Patch darin: review+

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.