Bug 36747 - Web Inspector: Placeholder for styles panel rendering experiments.
Summary: Web Inspector: Placeholder for styles panel rendering experiments.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-29 02:06 PDT by Pavel Feldman
Modified: 2010-05-11 19:32 PDT (History)
6 users (show)

See Also:


Attachments
[IMAGE] Firebug-alike version. (123.62 KB, image/png)
2010-03-29 02:08 PDT, Pavel Feldman
no flags Details
[IMAGE] Screenshot while running with patch. (140.62 KB, image/jpeg)
2010-03-29 09:35 PDT, Pavel Feldman
no flags Details
[IMAGE] Screenshot while running with patch. (133.34 KB, image/png)
2010-03-29 12:44 PDT, Pavel Feldman
no flags Details
[IMAGE] Screenshot while running with patch (2) (124.51 KB, image/png)
2010-03-29 12:45 PDT, Pavel Feldman
no flags Details
[PATCH] Proposed change. (24.60 KB, patch)
2010-03-29 12:47 PDT, Pavel Feldman
no flags Details | Formatted Diff | Diff
[IMAGE] Web Inspector mockup from Lennart (23.78 KB, image/png)
2010-04-18 20:12 PDT, Pavel Feldman
no flags Details
[IMAGE] Screenshot while running with patch (updated). (110.92 KB, image/png)
2010-05-10 11:52 PDT, Pavel Feldman
no flags Details
[PATCH] Rebaselined change. (25.69 KB, patch)
2010-05-10 11:54 PDT, Pavel Feldman
no flags Details | Formatted Diff | Diff
[PATCH] Same with binary diff. (25.80 KB, patch)
2010-05-10 12:56 PDT, Pavel Feldman
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-03-29 02:06:30 PDT
I'll be posting screenshots here.
Comment 1 Pavel Feldman 2010-03-29 02:08:29 PDT
Created attachment 51887 [details]
[IMAGE] Firebug-alike version.

(Computed style is a Styles' sibling below the screen).
Comment 2 Pavel Feldman 2010-03-29 09:35:26 PDT
Created attachment 51923 [details]
[IMAGE] Screenshot while running with patch.
Comment 3 Pavel Feldman 2010-03-29 12:44:49 PDT
Created attachment 51948 [details]
[IMAGE] Screenshot while running with patch.
Comment 4 Pavel Feldman 2010-03-29 12:45:52 PDT
Created attachment 51949 [details]
[IMAGE] Screenshot while running with patch (2)
Comment 5 Pavel Feldman 2010-03-29 12:47:23 PDT
Created attachment 51950 [details]
[PATCH] Proposed change.
Comment 6 Pavel Feldman 2010-04-01 13:51:06 PDT
Comment on attachment 51950 [details]
[PATCH] Proposed change.

We will redesign / land later.
Comment 7 Pavel Feldman 2010-04-18 20:11:24 PDT
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
Comment 8 Pavel Feldman 2010-04-18 20:12:45 PDT
Created attachment 53651 [details]
[IMAGE] Web Inspector mockup from Lennart
Comment 9 Pavel Feldman 2010-05-10 11:52:27 PDT
Created attachment 55583 [details]
[IMAGE] Screenshot while running with patch (updated).
Comment 10 Pavel Feldman 2010-05-10 11:54:49 PDT
Created attachment 55584 [details]
[PATCH] Rebaselined change.
Comment 11 Chris Jerdonek 2010-05-10 12:47:38 PDT
(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
Comment 12 Pavel Feldman 2010-05-10 12:56:52 PDT
Created attachment 55594 [details]
[PATCH] Same with binary diff.

Are you saying CQ is now capable of landing git binary diffs as well? Nice!
Comment 13 Chris Jerdonek 2010-05-10 13:02:20 PDT
(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. :)
Comment 14 Chris Jerdonek 2010-05-10 13:12:04 PDT
(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".
Comment 15 Joseph Pecoraro 2010-05-10 13:18:09 PDT
> 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. =/
Comment 16 Pavel Feldman 2010-05-10 13:28:02 PDT
> 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.
Comment 17 Chris Jerdonek 2010-05-10 13:44:28 PDT
(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
Comment 18 Yury Semikhatsky 2010-05-11 05:37:29 PDT
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.
Comment 19 Pavel Feldman 2010-05-11 07:15:42 PDT
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
Comment 20 Simon Fraser (smfr) 2010-05-11 16:53:10 PDT
Inspector is no longer showing any computed style for some elements for me now.
Comment 21 Joseph Pecoraro 2010-05-11 19:32:36 PDT
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!