Bug 91174

Summary: Comment on WebCore::HTMLMediaElement::childShouldCreateRenderer() should explain why
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: DOMAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, feature-media-reviews, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Hajime Morrita
Reported 2012-07-12 17:38:42 PDT
This is a followup of Bug 91055.
Attachments
Patch (1.88 KB, patch)
2012-07-12 18:24 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch (1.80 KB, patch)
2012-07-13 00:24 PDT, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2012-07-12 18:24:04 PDT
Created attachment 152124 [details] Patch
Shinya Kawanaka
Comment 2 2012-07-12 18:26:18 PDT
Comment on attachment 152124 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152124&action=review > Source/WebCore/ChangeLog:8 > + Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!). ?
Kent Tamura
Comment 3 2012-07-12 20:13:44 PDT
Comment on attachment 152124 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152124&action=review > Source/WebCore/html/HTMLMediaElement.cpp:494 > + // be rendered. So this should return true for most of the children. should return false?
Hajime Morrita
Comment 4 2012-07-13 00:24:09 PDT
Created attachment 152168 [details] Patch
Hajime Morrita
Comment 5 2012-07-13 00:25:15 PDT
Comment on attachment 152124 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=152124&action=review >> Source/WebCore/html/HTMLMediaElement.cpp:494 >> + // be rendered. So this should return true for most of the children. > > should return false? Ooops. Thanks for the catch. Just updated the patch. Tried to clarified the comment/made it wrong (your favorite meme here)...
Kent Tamura
Comment 6 2012-07-13 01:57:22 PDT
Comment on attachment 152168 [details] Patch Looks ok.
Eric Carlson
Comment 7 2012-07-13 10:28:12 PDT
Comment on attachment 152168 [details] Patch I get it now, thanks for taking the time to update the comment!
WebKit Review Bot
Comment 8 2012-07-16 17:49:07 PDT
Comment on attachment 152168 [details] Patch Clearing flags on attachment: 152168 Committed r122786: <http://trac.webkit.org/changeset/122786>
WebKit Review Bot
Comment 9 2012-07-16 17:49:14 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.