Bug 235454 - Web Inspector: [Flexbox] Add instrumentation/protocol bits for flex layout containers
Summary: Web Inspector: [Flexbox] Add instrumentation/protocol bits for flex layout co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Patrick Angle
URL:
Keywords: InRadar
Depends on:
Blocks: 235647
  Show dependency treegraph
 
Reported: 2022-01-21 14:11 PST by Patrick Angle
Modified: 2022-01-26 09:31 PST (History)
15 users (show)

See Also:


Attachments
Patch v1.0 (10.61 KB, patch)
2022-01-21 14:20 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Patch v1.1 - Address review nits (10.72 KB, patch)
2022-01-21 16:21 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Patch v1.2 - Resolve debug asserts (11.07 KB, patch)
2022-01-24 08:37 PST, Patrick Angle
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Angle 2022-01-21 14:11:53 PST
<rdar://87893199>
Comment 1 Patrick Angle 2022-01-21 14:20:34 PST
Created attachment 449695 [details]
Patch v1.0
Comment 2 EWS Watchlist 2022-01-21 14:22:29 PST
This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features.
Comment 3 Devin Rousso 2022-01-21 15:31:56 PST
Comment on attachment 449695 [details]
Patch v1.0

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

r=me, neat :)

I'm guessing the UI and other things will be coming later?

> Source/WebCore/inspector/agents/InspectorCSSAgent.cpp:945
> +    if (is<RenderFlexibleBox>(renderer))
> +        return Protocol::CSS::LayoutContextType::Flex;

NIT: I'd put this before `if (is<RenderGrid>(renderer))` so that it is alphabetical and matches the order in the protocol :P

> Source/WebCore/rendering/RenderFlexibleBox.cpp:76
>      setChildrenInline(false); // All of our children must be block-level.
> +    InspectorInstrumentation::nodeLayoutContextChanged(element, this);

Style: I'd add a newline

> Source/WebCore/rendering/RenderFlexibleBox.cpp:83
>      setChildrenInline(false); // All of our children must be block-level.
> +    InspectorInstrumentation::nodeLayoutContextChanged(document, this);

ditto (:75)
Comment 4 Patrick Angle 2022-01-21 16:21:28 PST
Created attachment 449708 [details]
Patch v1.1 - Address review nits
Comment 5 Patrick Angle 2022-01-24 08:37:23 PST
Created attachment 449820 [details]
Patch v1.2 - Resolve debug asserts
Comment 6 EWS 2022-01-24 17:12:29 PST
Committed r288492 (246364@main): <https://commits.webkit.org/246364@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449820 [details].