Bug 148674 – Web Inspector: Show layout/paint pixel area in the Rendering Frames tree outline

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

[Matt Baker]

Created attachment 260357 [details]
[Image] New UI

* SUMMARY
Show layout/paint pixel area in the Rendering Frames tree outline. Since the data grid lacks 'width' and 'height' columns, we can add a tree element subtitle for timeline records that include pixel area.

* NOTES
We should also highlight the region in the inspected page corresponding to the pixel area of a hovered layout or paint record, like the Layout timeline view. See https://bugs.webkit.org/show_bug.cgi?id=148673.

[Radar WebKit Bug Importer]

<rdar://problem/22515649>

[Matt Baker]

Created attachment 260358 [details]
[Patch] Proposed Fix

[Blaze Burg]

What is shown in the screenshot are the dimensions, not the area (i.e., px^2). I think the dimensions are fine for the subtitle. Will this change also affect subtitle in layout timeline view?


It would be nice to add back in an "area" column to the layout timeline view. I'll file a bug about that.

[Blaze Burg]

Comment on attachment 260358 [details]
[Patch] Proposed Fix

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

r=me. I guess we don't want this in layout timeline view since it's already in the columns there, but not in rendering timeline.

> Source/WebInspectorUI/UserInterface/Views/RenderingFrameTimelineView.js:276
> +                        subtitle.textContent = WebInspector.UIString("%d x %d").format(childRecord.width, childRecord.height);

I would use a unicode X instead of an 'x' character. See https://en.wikipedia.org/wiki/X_mark. We may already be using this somewhere. In my testing, U+2A09 looks the best on 10.10, but you should test on 10.11 too.

[Matt Baker]

(In reply to comment #3)
> What is shown in the screenshot are the dimensions, not the area (i.e.,
> px^2). I think the dimensions are fine for the subtitle. Will this change
> also affect subtitle in layout timeline view?

This won't effect the layout timeline view.

[Matt Baker]

Created attachment 260385 [details]
[Image] Using n-ary times operator

[Matt Baker]

Created attachment 260386 [details]
[Patch] Proposed Fix

[Timothy Hatcher]

Comment on attachment 260386 [details]
[Patch] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/RenderingFrameTimelineView.js:276
> +                        subtitle.textContent = WebInspector.UIString("%d ⨠%d").format(childRecord.width, childRecord.height);

You should use \u instead of the literal UTF-8 here.

[Matt Baker]

(In reply to comment #8)
> Comment on attachment 260386 [details]
> [Patch] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=260386&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/RenderingFrameTimelineView.js:276
> > +                        subtitle.textContent = WebInspector.UIString("%d ⨠%d").format(childRecord.width, childRecord.height);
> 
> You should use \u instead of the literal UTF-8 here.

Should we always prefer \u, or just when the literal could be easily confused with another character?

[Matt Baker]

Created attachment 260389 [details]
[Patch] Proposed Fix

[Timothy Hatcher]

(In reply to comment #9)
> (In reply to comment #8)
> > Comment on attachment 260386 [details]
> > [Patch] Proposed Fix
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=260386&action=review
> > 
> > > Source/WebInspectorUI/UserInterface/Views/RenderingFrameTimelineView.js:276
> > > +                        subtitle.textContent = WebInspector.UIString("%d ⨠%d").format(childRecord.width, childRecord.height);
> > 
> > You should use \u instead of the literal UTF-8 here.
> 
> Should we always prefer \u, or just when the literal could be easily
> confused with another character?

It is good to do when it isn't a strict ASCII character. No all tools or editors like Unicode.

[WebKit Commit Bot]

Comment on attachment 260389 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 260389

Committed r189236: <http://trac.webkit.org/changeset/189236>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.