Bug 121719 – Web Inspector: [CSS Regions] Display CSS Regions chain when highlighting a CSS Region node

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Alexandru Chiculita]

Add code in the InspectorOverlay to highlight the chain of CSS Regions.

[Radar WebKit Bug Importer]

<rdar://problem/15044651>

[Alexandru Chiculita]

Created attachment 212222 [details]
Patch V1

[Alexandru Chiculita]

Created attachment 212223 [details]
Screenshot

[Joseph Pecoraro]

Comment on attachment 212222 [details]
Patch V1

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

r=me! A few optional nits, feel free to cq+.

> Source/WebCore/ChangeLog:11
> +        When a CSS Region node is highlighted through the WebInspector, it will also lookup all the regions
> +        part of the same flow and inject enough information into InspectorOverlayPage.js to get the other

Nit: "all the regions part of the same flow" => "all the regions that are part of the same flow"

> Source/WebCore/inspector/InspectorOverlay.cpp:399
> +static void buildObjectForCSSRegionsHighlight(Node* node, InspectorObject* nodeObject)

Nit: I think naming "nodeObject" => "highlightNodeObject" would be clearer.

> Source/WebCore/inspector/InspectorOverlayPage.js:428
> +    }
> +
> +    for (var i = 0; i < regions.length; ++i) {
> +        var region = regions[i];

Nit: These top two loops can be combined without losing clarity. These lines could just be removed.

> Source/WebCore/testing/Internals.cpp:746
> +    RefPtr<InspectorObject> object = document->page()->inspectorController()->buildObjectForHighlightedNode();
> +    return object ? object->toJSONString() : String();

This JSON stringify/parse seems unnecessary. It seems the IDL for this could return a nullable object here instead of a string <http://www.w3.org/TR/WebIDL/#idl-object>:

    [RaisesException] object? inspectorHighlightObject(Document document);

However, having said that, I don't know what syntax would need to be here. It might even need to be Custom in that case. So I think what you have is fine.

[Alexandru Chiculita]

Created attachment 212244 [details]
Patch V2

[Alexandru Chiculita]

Created attachment 212247 [details]
Patch V3

Added reviewer in Changelogs this time.

[WebKit Commit Bot]

Comment on attachment 212247 [details]
Patch V3

Clearing flags on attachment: 212247

Committed r156215: <http://trac.webkit.org/changeset/156215>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.