Bug 36747 – Web Inspector: Placeholder for styles panel rendering experiments.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

I'll be posting screenshots here.

[Pavel Feldman]

Created attachment 51887 [details]
[IMAGE] Firebug-alike version.

(Computed style is a Styles' sibling below the screen).

[Pavel Feldman]

Created attachment 51923 [details]
[IMAGE] Screenshot while running with patch.

[Pavel Feldman]

Created attachment 51948 [details]
[IMAGE] Screenshot while running with patch.

[Pavel Feldman]

Created attachment 51949 [details]
[IMAGE] Screenshot while running with patch (2)

[Pavel Feldman]

Created attachment 51950 [details]
[PATCH] Proposed change.

[Pavel Feldman]

Comment on attachment 51950 [details]
[PATCH] Proposed change.

We will redesign / land later.

[Pavel Feldman]

I got this via email:

Hi, thanks for commenting on my blog post about my Web Inspector wish
list. I took a good look at your screenshot from bug #36747, and I
really like it, but I'd go even further. I've Photoshopped your patch
screenshot and attached it. I realize it is almost a carbon copy of
Firebug, but it's with reason. Please follow along:

- Moved the big sections to tabs. It makes a lot more sense to me, and
they're much easier to find this way. We would however have to come up
with a solution for the amount of tabs and the limited space.
- Added a bit more spacing everywhere and removed the grey lines
between selectors – allows everything to "breathe" a little more.
- Removed all underlined links, they are all blue anyway, and this
further reduces clutter.
- Removed the element styles section, they should go on top of
matching rules. I like the Firebug way of only showing them when there
are any, and using an option in the contextual menu to add one.
- Removed user agent styles, as per my request from
https://bugs.webkit.org/show_bug.cgi?id=37766

Combine that with
- putting the checkbox to disable/enable styles to the left of the
property (https://bugs.webkit.org/show_bug.cgi?id=37770)
- remembering split pane positions
(https://bugs.webkit.org/show_bug.cgi?id=37767)
- a slightly bigger font-size perhaps

... and I'd say it's a worthy competitor to Firebug. :)

So, what do you think?

Thanks a lot!
Lennart

[Pavel Feldman]

Created attachment 53651 [details]
[IMAGE] Web Inspector mockup from Lennart

[Pavel Feldman]

Created attachment 55583 [details]
[IMAGE] Screenshot while running with patch (updated).

[Pavel Feldman]

Created attachment 55584 [details]
[PATCH] Rebaselined change.

[Chris Jerdonek]

(In reply to comment #10)
> Created an attachment (id=55584) [details]
> [PATCH] Rebaselined change.

You need to use the --binary flag with git-diff:

http://webkit-commit-queue.appspot.com/results/2214120

[Pavel Feldman]

Created attachment 55594 [details]
[PATCH] Same with binary diff.

Are you saying CQ is now capable of landing git binary diffs as well? Nice!

[Chris Jerdonek]

(In reply to comment #12)
> Created an attachment (id=55594) [details]
> [PATCH] Same with binary diff.
> 
> Are you saying CQ is now capable of landing git binary diffs as well? Nice!

As of several months ago I think.  I wasn't responsible for that addition though -- just the nicer error message. :)

[Chris Jerdonek]

(In reply to comment #12)
> Created an attachment (id=55594) [details]
> [PATCH] Same with binary diff.
> 
> Are you saying CQ is now capable of landing git binary diffs as well? Nice!

Oh, well, you tried.  It doesn't completely support it yet:

Failed to run "[u'/mnt/git/webkit-style-queue/WebKitTools/Scripts/svn-apply', u'--force']" exit_code: 9
Parsed 6 diffs from patch file(s).
patching file LayoutTests/inspector/elements-panel-styles-expected.txt
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
only literal type is supported now at /mnt/git/webkit-style-queue/WebKitTools/Scripts/svn-apply line 256.

https://webkit-commit-queue.appspot.com/results/2166128

It's a FIXME for svn-apply to support git binary diffs having the "delta" format.  I'm not sure if there is a way to force the use of "literal".

[Joseph Pecoraro]

> It's a FIXME for svn-apply to support git binary diffs having the "delta" format.
> I'm not sure if there is a way to force the use of "literal".

A little while ago I asked in #git and looked at the git source code. There currently is no way to force it to use literal. =/

[Pavel Feldman]

> Oh, well, you tried.  It doesn't completely support it yet:

Heh. That's where I thought we were. That is why I am not cooking binary diffs and land binary-related stuff by hand.

[Chris Jerdonek]

(In reply to comment #16)
> > Oh, well, you tried.  It doesn't completely support it yet:
> 
> Heh. That's where I thought we were. That is why I am not cooking binary diffs and land binary-related stuff by hand.

I filed a bug for this here:

https://bugs.webkit.org/show_bug.cgi?id=38864

[Yury Semikhatsky]

Comment on attachment 55594 [details]
[PATCH] Same with binary diff.

WebCore/inspector/front-end/StylesSidebarPane.js:639
 +      collapse: function(dontRememberState)
Maybe remove this too?

The patch looks sane.

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/inspector/elements-panel-styles-expected.txt
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/inspector/InspectorDOMAgent.cpp
	M	WebCore/inspector/front-end/Checkbox.js
	M	WebCore/inspector/front-end/ElementsPanel.js
	M	WebCore/inspector/front-end/StylesSidebarPane.js
	M	WebCore/inspector/front-end/WorkersSidebarPane.js
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/inspector/front-end/treeoutline.js
Committed r59150

[Simon Fraser (smfr)]

Inspector is no longer showing any computed style for some elements for me now.

[Joseph Pecoraro]

A patch like this could really use more detail in the ChangeLog. If not when it goes into Bugzilla (via an automated tool like webkit-patch) then most certainly before landing!