Bug 62411

Summary: REGRESSION: End of apple.com video in full-screen mode leads to unusable page.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: MediaAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

Jer Noble
Reported 2011-06-09 16:06:51 PDT
REGRESSION: End of apple.com video in full-screen mode leads to unusable page.
Attachments
Patch (6.00 KB, patch)
2011-06-09 16:18 PDT, Jer Noble
darin: review+
Jer Noble
Comment 1 2011-06-09 16:18:41 PDT
WebKit Review Bot
Comment 2 2011-06-09 16:19:45 PDT
Attachment 96659 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/rendering/RenderFullScreen.cpp:49: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Jer Noble
Comment 3 2011-06-10 08:07:57 PDT
Darin Adler
Comment 4 2011-06-10 09:31:54 PDT
Comment on attachment 96659 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96659&action=review > Source/WebCore/rendering/RenderFullScreen.cpp:47 > + virtual ~RenderFullScreenPlaceholder() { } There’s no need for you to define this explicitly. > Source/WebCore/rendering/RenderFullScreen.cpp:48 > + virtual bool isRenderFullScreenPlaceholder() const { return true; } This could be private. >> Source/WebCore/rendering/RenderFullScreen.cpp:49 >> + virtual void destroy() { > > Place brace on its own line for function definitions. [whitespace/braces] [4] I think this could be private. I think it would be better style to not make this inline. > Source/WebCore/rendering/RenderFullScreen.cpp:53 > + RenderFullScreen* m_owner; This could be private.
Darin Adler
Comment 5 2011-06-10 09:32:13 PDT
Where’s the regression test?
Jer Noble
Comment 6 2011-06-10 09:43:06 PDT
(In reply to comment #5) > Where’s the regression test? I'll write one up before landing. Thanks!
Jer Noble
Comment 7 2011-06-10 20:36:38 PDT
Note You need to log in before you can comment on or make changes to this bug.