Bug 143309 - Web Inspector: ObjectTree array index hints are clipped when shown in popover
Summary: Web Inspector: ObjectTree array index hints are clipped when shown in popover
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-04-01 10:17 PDT by Brian Burg
Modified: 2015-04-06 12:58 PDT (History)
8 users (show)

See Also:


Attachments
screenshot of clipped values (36.02 KB, image/png)
2015-04-01 10:18 PDT, Brian Burg
no flags Details
Reduction (205 bytes, text/html)
2015-04-04 18:31 PDT, Nikita Vasilyev
no flags Details
Patch (1.23 KB, patch)
2015-04-04 19:04 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff
Screenshot with the patch applied (152.54 KB, image/png)
2015-04-04 19:06 PDT, Nikita Vasilyev
no flags Details
Map before/after (37.50 KB, image/gif)
2015-04-06 08:27 PDT, Nikita Vasilyev
no flags Details
[Animated GIF] Safari 8/before the patch/after the patch (24.38 KB, image/gif)
2015-04-06 08:58 PDT, Nikita Vasilyev
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Burg 2015-04-01 10:17:29 PDT
Oops
Comment 1 Radar WebKit Bug Importer 2015-04-01 10:17:56 PDT
<rdar://problem/20384458>
Comment 2 Brian Burg 2015-04-01 10:18:02 PDT
Created attachment 249930 [details]
screenshot of clipped values
Comment 3 Nikita Vasilyev 2015-04-04 18:31:14 PDT
Created attachment 250141 [details]
Reduction
Comment 4 Nikita Vasilyev 2015-04-04 19:04:50 PDT
Created attachment 250143 [details]
Patch
Comment 5 Nikita Vasilyev 2015-04-04 19:06:57 PDT
Created attachment 250144 [details]
Screenshot with the patch applied

The red bars on the screenshot are purely for debugging and aren't present in the patch.

    .object-tree-array-index .index-name {outline: 1px solid red;}
Comment 6 Brian Burg 2015-04-05 09:07:00 PDT
Comment on attachment 250143 [details]
Patch

r=me
Comment 7 Timothy Hatcher 2015-04-05 09:29:00 PDT
Comment on attachment 250143 [details]
Patch

Is Set and Map also broken?
Comment 8 WebKit Commit Bot 2015-04-05 09:53:42 PDT
Comment on attachment 250143 [details]
Patch

Clearing flags on attachment: 250143

Committed r182359: <http://trac.webkit.org/changeset/182359>
Comment 9 WebKit Commit Bot 2015-04-05 09:53:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Joseph Pecoraro 2015-04-05 12:41:33 PDT
(In reply to comment #7)
> Comment on attachment 250143 [details]
> Patch
> 
> Is Set and Map also broken?

Also, are these popover only issues or are there regressions in the console as well?
Comment 11 Nikita Vasilyev 2015-04-05 17:59:13 PDT
(In reply to comment #10)
> (In reply to comment #7)
> > Comment on attachment 250143 [details]
> > Patch
> > 
> > Is Set and Map also broken?

Set was clipped, now it’s fine.

Can’t test map at the moment, it’s broken: https://bugs.webkit.org/show_bug.cgi?id=143428

> Also, are these popover only issues or are there regressions in the console
> as well?

In the console it wasn't clipped because there was enough extra space on the left.
Comment 12 Joseph Pecoraro 2015-04-05 18:05:23 PDT
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #7)
> > > Comment on attachment 250143 [details]
> > > Patch
> > > 
> > > Is Set and Map also broken?
> 
> Set was clipped, now it’s fine.
> 
> Can’t test map at the moment, it’s broken:
> https://bugs.webkit.org/show_bug.cgi?id=143428

That has a patch. You can apply that patch and test.

> > Also, are these popover only issues or are there regressions in the console
> > as well?
> 
> In the console it wasn't clipped because there was enough extra space on the
> left.

The styles I had matched http://timothy.hatcher.name/console/ pretty closely. Do they still match?
Comment 13 Timothy Hatcher 2015-04-06 05:59:48 PDT
(In reply to comment #12)
> > In the console it wasn't clipped because there was enough extra space on the
> > left.
> 
> The styles I had matched http://timothy.hatcher.name/console/ pretty
> closely. Do they still match?

It looks like the indices no long line up under the disclosure triangle, but under the Array icon in the console. That is okay with me. But it likely should shift to the right more in that case to like up more in the center of the icon.

I'm more curious how it looks next to other objects, etc. If it isn't garishly indented now, it should be fine.
Comment 14 Nikita Vasilyev 2015-04-06 08:27:27 PDT
Created attachment 250205 [details]
Map before/after

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > (In reply to comment #7)
> > > > Comment on attachment 250143 [details]
> > > > Patch
> > > > 
> > > > Is Set and Map also broken?
> > 
> > Set was clipped, now it’s fine.
> > 
> > Can’t test map at the moment, it’s broken:
> > https://bugs.webkit.org/show_bug.cgi?id=143428
> 
> That has a patch. You can apply that patch and test.

Map was clipped too.
Comment 15 Nikita Vasilyev 2015-04-06 08:58:13 PDT
Created attachment 250206 [details]
[Animated GIF] Safari 8/before the patch/after the patch

While I do like how on http://timothy.hatcher.name/console/ the indices are aligned under ▼, they don’t work well when nested inside an object.

    console.dir({
      b: ["Zoidberg", "bar", "baz"],
      property: "Zoidberg"
    })

Take a look at the GIF. It may look like `property: "Zoidberg"` is in Array Prototype. We may want to increase the left margin even further to fix it. What do you think?


(In reply to comment #12)
> The styles I had matched http://timothy.hatcher.name/console/ pretty
> closely. Do they still match?

They don’t, intentionally. See above.
Comment 16 Timothy Hatcher 2015-04-06 12:58:07 PDT
Yeah, I think we might want a few pixels more margin. At least line up the characters so monospace values make sense.