Bug 141647 - REGRESSION(r180050): It broke the !ENABLE(CSS_GRID_LAYOUT) build
Summary: REGRESSION(r180050): It broke the !ENABLE(CSS_GRID_LAYOUT) build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: ChangSeok Oh
URL:
Keywords:
Depends on:
Blocks: 141465
  Show dependency treegraph
 
Reported: 2015-02-16 08:07 PST by Csaba Osztrogonác
Modified: 2015-02-17 04:02 PST (History)
6 users (show)

See Also:


Attachments
Patch (2.12 KB, patch)
2015-02-16 11:15 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (1.84 KB, patch)
2015-02-16 19:28 PST, ChangSeok Oh
ossy: review+
ossy: commit-queue-
Details | Formatted Diff | Diff
Patch (1.84 KB, patch)
2015-02-17 00:32 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (1.79 KB, patch)
2015-02-17 00:35 PST, ChangSeok Oh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2015-02-16 08:07:34 PST
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
Comment 1 ChangSeok Oh 2015-02-16 08:22:21 PST
Oops. Sorry for the inconvenience. If nobody is touching on this, let me fix it soon.
Comment 2 ChangSeok Oh 2015-02-16 11:15:19 PST
Created attachment 246664 [details]
Patch
Comment 3 WebKit Commit Bot 2015-02-16 11:17:58 PST
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.
Comment 4 Csaba Osztrogonác 2015-02-16 12:07:16 PST
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)) {
...
Comment 5 ChangSeok Oh 2015-02-16 19:26:32 PST
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.
Comment 6 ChangSeok Oh 2015-02-16 19:28:53 PST
Created attachment 246717 [details]
Patch
Comment 7 Csaba Osztrogonác 2015-02-16 23:36:38 PST
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
Comment 8 ChangSeok Oh 2015-02-17 00:32:00 PST
Created attachment 246734 [details]
Patch
Comment 9 ChangSeok Oh 2015-02-17 00:35:43 PST
Created attachment 246735 [details]
Patch
Comment 10 WebKit Commit Bot 2015-02-17 01:42:24 PST
Comment on attachment 246735 [details]
Patch

Clearing flags on attachment: 246735

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