Bug 148674

Summary: Web Inspector: Show layout/paint pixel area in the Rendering Frames tree outline
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Image] New UI
none
[Patch] Proposed Fix
none
[Image] Using n-ary times operator
none
[Patch] Proposed Fix
none
[Patch] Proposed Fix none

Description Matt Baker 2015-09-01 02:09:07 PDT
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.
Comment 1 Radar WebKit Bug Importer 2015-09-01 02:09:24 PDT
<rdar://problem/22515649>
Comment 2 Matt Baker 2015-09-01 02:14:55 PDT
Created attachment 260358 [details]
[Patch] Proposed Fix
Comment 3 BJ Burg 2015-09-01 08:30:40 PDT
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.
Comment 4 BJ Burg 2015-09-01 08:39:38 PDT
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.
Comment 5 Matt Baker 2015-09-01 10:38:42 PDT
(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.
Comment 6 Matt Baker 2015-09-01 14:42:28 PDT
Created attachment 260385 [details]
[Image] Using n-ary times operator
Comment 7 Matt Baker 2015-09-01 14:45:24 PDT
Created attachment 260386 [details]
[Patch] Proposed Fix
Comment 8 Timothy Hatcher 2015-09-01 15:22:27 PDT
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.
Comment 9 Matt Baker 2015-09-01 15:38:05 PDT
(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?
Comment 10 Matt Baker 2015-09-01 15:45:58 PDT
Created attachment 260389 [details]
[Patch] Proposed Fix
Comment 11 Timothy Hatcher 2015-09-01 16:40:16 PDT
(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.
Comment 12 WebKit Commit Bot 2015-09-01 17:25:49 PDT
Comment on attachment 260389 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 260389

Committed r189236: <http://trac.webkit.org/changeset/189236>
Comment 13 WebKit Commit Bot 2015-09-01 17:25:55 PDT
All reviewed patches have been landed.  Closing bug.