Bug 141647 – REGRESSION(r180050): It broke the !ENABLE(CSS_GRID_LAYOUT) build

• Component: New Bugs
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

error:
../../Source/WebCore/dom/Position.cpp: In member function 'bool WebCore::Position::isCandidate() const':
../../Source/WebCore/dom/Position.cpp:940:82: error: 'RenderGrid' was not declared in this scope

[ChangSeok Oh]

Oops. Sorry for the inconvenience. If nobody is touching on this, let me fix it soon.

[ChangSeok Oh]

Created attachment 246664 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/dom/Position.cpp:943:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 2 files


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

[Csaba Osztrogonác]

Comment on attachment 246664 [details]
Patch

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

> Source/WebCore/dom/Position.cpp:56
> +

No need to guard the include, RenderGrid.h is already guarded.

> Source/WebCore/dom/Position.cpp:947
> -    if (is<RenderBlockFlow>(*renderer) || is<RenderFlexibleBox>(*renderer) || is<RenderGrid>(*renderer)) {
> +    if (is<RenderBlockFlow>(*renderer) || is<RenderFlexibleBox>(*renderer)
> +#if ENABLE(CSS_GRID_LAYOUT)
> +        || is<RenderGrid>(*renderer)
> +#endif
> +        ) {

Ouch, it is so bad looking.

Maybe this one would be a little bit better, but still ugly:

if (is<RenderBlockFlow>(*renderer) || 
#if ENABLE(CSS_GRID_LAYOUT)
    is<RenderGrid>(*renderer) ||
#endif
    is<RenderFlexibleBox>(*renderer)) {
...

[ChangSeok Oh]

Comment on attachment 246664 [details]
Patch

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

Thanks for your comments. =)

>> Source/WebCore/dom/Position.cpp:56
>> +
> 
> No need to guard the include, RenderGrid.h is already guarded.

O.K Removed.

>> Source/WebCore/dom/Position.cpp:947
>> +        ) {
> 
> Ouch, it is so bad looking.
> 
> Maybe this one would be a little bit better, but still ugly:
> 
> if (is<RenderBlockFlow>(*renderer) || 
> #if ENABLE(CSS_GRID_LAYOUT)
>     is<RenderGrid>(*renderer) ||
> #endif
>     is<RenderFlexibleBox>(*renderer)) {
> ...

Indeed. However a similar ugly look already exists. https://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderBox.cpp#L2373 :P
Let me update the patch as you noted.

[ChangSeok Oh]

Created attachment 246717 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 246717 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:10
> +        * dom/Position.cpp: Add a build gard ENABLE(CSS_GRID_LAYOUT) for RenderGrid.

typo: gard -> guard

[ChangSeok Oh]

Created attachment 246734 [details]
Patch

[ChangSeok Oh]

Created attachment 246735 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 246735 [details]
Patch

Clearing flags on attachment: 246735

Committed r180213: <http://trac.webkit.org/changeset/180213>