Bug 137641

Summary:

Use is<>() / downcast<>() for Table render objects

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

Layout and Rendering

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, commit-queue, darin, kling, rniwa

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

137424

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none

Description Chris Dumez 2014-10-11 17:49:59 PDT

Use is<>() / downcast<>() for Table render objects.

Comment 1 Chris Dumez 2014-10-11 17:58:22 PDT

Created attachment 239692 [details]
Patch

Comment 2 WebKit Commit Bot 2014-10-11 17:59:23 PDT

Attachment 239692 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:470:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:492:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 2 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 3 Mihnea Ovidenie 2014-10-13 08:04:52 PDT

Comment on attachment 239692 [details]
Patch

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

r=me with the two comments

> Source/WebCore/rendering/AutoTableLayout.cpp:192
> +            RenderBlock* renderBlock = table->containingBlock();

I believe containingBlock is better than renderBlock here if you want to change it from cb.

> Source/WebCore/rendering/RenderTableCol.cpp:33
> +#include "RenderTableCaption.h"

Is this include needed?

Comment 4 Chris Dumez 2014-10-13 10:22:35 PDT

Comment on attachment 239692 [details]
Patch

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

>> Source/WebCore/rendering/AutoTableLayout.cpp:192
>> +            RenderBlock* renderBlock = table->containingBlock();
> 
> I believe containingBlock is better than renderBlock here if you want to change it from cb.

Sounds good.

>> Source/WebCore/rendering/RenderTableCol.cpp:33
>> +#include "RenderTableCaption.h"
> 
> Is this include needed?

Yes, see below.

> Source/WebCore/rendering/RenderTableCol.cpp:174
> +        if (is<RenderTableCaption>(*next))

The include is needed because of this check. RenderTableCaption type and its template specialization need to be defined.

Comment 5 Chris Dumez 2014-10-13 10:24:28 PDT

Created attachment 239731 [details]
Patch

Comment 6 WebKit Commit Bot 2014-10-13 10:27:24 PDT

Attachment 239731 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:470:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/rendering/RenderTreeAsText.cpp:492:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 2 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 7 WebKit Commit Bot 2014-10-13 11:21:12 PDT

Comment on attachment 239731 [details]
Patch

Clearing flags on attachment: 239731

Committed r174653: <http://trac.webkit.org/changeset/174653>

Comment 8 WebKit Commit Bot 2014-10-13 11:21:17 PDT

All reviewed patches have been landed.  Closing bug.