Bug 143309

Summary: Web Inspector: ObjectTree array index hints are clipped when shown in popover
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Nikita Vasilyev <nvasilyev>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
screenshot of clipped values
none
Reduction
none
Patch
none
Screenshot with the patch applied
none
Map before/after
none
[Animated GIF] Safari 8/before the patch/after the patch none

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.