Bug 148674 - Web Inspector: Show layout/paint pixel area in the Rendering Frames tree outline
Summary: Web Inspector: Show layout/paint pixel area in the Rendering Frames tree outline
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: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-09-01 02:09 PDT by Matt Baker
Modified: 2015-09-01 17:25 PDT (History)
8 users (show)

See Also:


Attachments
[Image] New UI (271.81 KB, image/png)
2015-09-01 02:09 PDT, Matt Baker
no flags Details
[Patch] Proposed Fix (4.15 KB, patch)
2015-09-01 02:14 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
[Image] Using n-ary times operator (79.33 KB, image/png)
2015-09-01 14:42 PDT, Matt Baker
no flags Details
[Patch] Proposed Fix (4.13 KB, patch)
2015-09-01 14:45 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
[Patch] Proposed Fix (4.15 KB, patch)
2015-09-01 15:45 PDT, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.