Bug 169082 – Enable fieldsets to be flex boxes and grids

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Dave Hyatt]

Enable fieldsets to be flex boxes and grids.

[Dave Hyatt]

Created attachment 303332 [details]
Patch

[Dave Hyatt]

*** Bug 148826 has been marked as a duplicate of this bug. ***

[Simon Fraser (smfr)]

Comment on attachment 303332 [details]
Patch

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

> Source/WebCore/rendering/OrderIterator.cpp:80
> +bool OrderIterator::shouldSkipChild(const RenderObject* child) const

This should take a reference.

> Source/WebCore/rendering/OrderIterator.h:75
> +    bool collectChild(const RenderBox&);

Please add a comment saying what the return value means.

> Source/WebCore/rendering/RenderBlock.cpp:1699
> +    

Trailing whitespace (gasp!)

> Source/WebCore/rendering/RenderBlock.cpp:2766
> +            int legendMinWidth = legend->minPreferredLogicalWidth();

Why int and not LayoutUnit? Darin would say that auto would have avoided this.

> Source/WebCore/rendering/RenderBlock.cpp:3939
> +    

Whitespace

> Source/WebCore/rendering/RenderBlock.h:400
> +    enum FieldsetFindLegendOption { FieldsetIgnoreFloatingOrOutOfFlow, FieldsetIncludeFloatingOrOutOfFlow };

enum class?

> Source/WebCore/rendering/RenderBlockFlow.h:475
> +    void layoutExcludedChildren(bool /*relayoutChildren*/) override;

No need to comment out the param name.

> Source/WebCore/rendering/RenderTable.cpp:782
> +    

whitespace

[Dave Hyatt]

Landed in r213379.

[Carlos Alberto Lopez Perez]

(In reply to comment #4)
> Landed in r213379.

This broke the GTK+ build:

Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:189:22: error: no viable conversion from 'WebCore::RenderElement' to 'WebCore::RenderBlock *'
        RenderBlock* renderFieldset = *coreObject->renderer()->parent();
                     ^                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.


https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29/builds/77905/steps/compile-webkit/logs/stdio

[Michael Catanzaro]

Windows too:

C:\cygwin\home\buildbot\WebKit\Source\WebCore\rendering\RenderingAllInOne.cpp(65): fatal error C1083: Cannot open include file: 'RenderFieldset.cpp': No such file or directory [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj]

[WebKit Commit Bot]

Re-opened since this is blocked by bug 169169

Bug 169169 has been closed - so this one can be closed as well?

[Dave Hyatt]

Created attachment 303518 [details]
Patch for EWS to make sure GTK builds

[Dave Hyatt]

(In reply to comment #8)
> Bug 169169 has been closed - so this one can be closed as well?

No, this got rolled out. Will re-land once I am sure it won't break GTK.

[Dave Hyatt]

Re-landed in 213455.

[Jon Lee]

rdar://problem/22607417