Bug 27124

Summary: let me *edit* css styles in the web inspector.
Product: WebKit Reporter: ryankshaw+webkitbugzilla
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abarth, eric.carlson, eric, joepeck, keishi, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Edit CSS Selectors
timothy: review-
Edit CSS Selectors (diff --binary) timothy: review+

Description ryankshaw+webkitbugzilla 2009-07-09 11:47:23 PDT
I would love to use safari for development but seriously the deal breaker is not being able to *edit* css styles. So I am proposing these features:

1. Let me edit the css styles for a specific element. (easily, like firebug lets me, not by having to try to add a style=”" attribute)

2. Let me edit the style properties of a css stylesheet rule, like in firebug. ex:
if I have
.some_selector {
color: red;
}
in my sylesheet, let me change that color to black or add “margin: 10px” or whatever I want to that rule.

3. This is a bonus, if you want to leapfrog firebug. Let me create stylesheet rules on the fly and assign properties to them. so, while I am looking at the page I can say “I wonder what would happen if gave all the h4s on the page an underline and increased their font size to 1.8em” and then I could just create a style h4{ text-decoration:underline; font-size: 1.8em;} and see it in action. (see http://flesler.blogspot.com/2007/11/jqueryrule.html for more of what I am talking about here)

With those, then I really would start using safari for development.
Comment 1 Keishi Hattori 2009-07-09 12:18:38 PDT
Web Inspector pretty much has the same css editing capability as Firebug does.

Edit is triggered by double clicking instead of a single click in Firebug. 
"Computed Style" is not a CSS rule so it is not editable. (Just as in Firebug)
Typing in multiple attributes will add the new attributes. Deleting all the text will delete the attribute.

Creating rules on the fly is a great idea. However style sheet rules have order dependency so I'm not sure how the interface will be like.
Comment 2 Mark Rowe (bdash) 2009-07-09 14:29:49 PDT
If we already have the functionality but we keep getting bug reports about us not having it, then it's a good sign that the functionality is not discoverable enough.  We should look in to how we can improve the UI so that users can find the functionality more easily.
Comment 3 ryankshaw+webkitbugzilla 2009-07-09 18:10:03 PDT
Keishi, first off: thanks a lot for the pointer, I am excited to see that editing css properties is, indeed, possible already by double clicking.  I would definately second Mark's suggestion that the user interaction on it could be a little more discoverable, as in dont make me double click, a single click should do.  while I am in edit mode if I hit tab, take me to a new line where I can create a new property for that style declaration.  
I made a video to make this more clear:
http://screencast.com/t/tlMxsfOUj

As far as creating stylesheet rules on the fly, you're right source order would be a factor here but I would suggest that if you created them on the fly it would be just like appending a <style> node right before </body> so it's source order would be last of all stylesheets.  
or in jquery code doing the equivalent of: 
$('<style>div.foo{color:blue;}</style>').appendTo('body'); 
where "div.foo{color:blue;}" was whatever they had typed in.

Along with this, something that would make us css dev's really happy is being able to modify the selector that was used for a certain style rule.  so if I had a selector: div.foo_class in my stylesheet, I could change that to be just .foo_class or h1.bar_class or whatever. 

here is a video to show those last 2 more clearly:
http://screencast.com/t/mJsGddXePE
Comment 4 Joseph Pecoraro 2009-07-26 23:41:30 PDT
Hey Ryan.  How does something like this look for a quick way to add new styles to a page.  Feedback from anyone would be appreciated:
http://screencast.com/t/GR79aGkGh

If it sounds like a good idea and the WebKit team likes it then I'll have to do some polishing up, most likely UI improvements and a better user experience then what I currently have.

Also, you might be interested in a recent patch that allows for "tabbing through css properties" that has been reviewed and will hopefully land soon (video examples in the comments):
https://bugs.webkit.org/show_bug.cgi?id=27673

I haven't looked into your last feature (editing the css selector itself) although that is something that I've wanted myself from time to time.
Comment 5 Joseph Pecoraro 2009-07-30 08:24:07 PDT
Created attachment 33783 [details]
Edit CSS Selectors

NOTES:

- Double-Click to Add CSS Selectors section, provides a useful initial value
- Double-Click a Selector to edit (if allowed)
- Double-Click White-Space to add a Property (enlarged whitespace)
- GUI Displays as Gray if an edit is Not Applicable to the current selected node
- Tabbing may go between the Selector and its Properties
- Strikethroughs and Ordering are accurate
- Selected Element's style attributes always show, expands automatically, otherwise collapsed
- Editing Element's style attributes is synced with Element's Tree Outline
- Fix for Adding a New Property and pushing Esc to Cancel
Comment 6 Joseph Pecoraro 2009-07-30 08:24:50 PDT
Added Tim to the CC list.
Comment 7 Timothy Hatcher 2009-07-30 08:32:43 PDT
Comment on attachment 33783 [details]
Edit CSS Selectors

> +        for (var i = 0, len = parentRules.length; i < len; ++i) {
> +        for (var i = 0, len = nodes.length; i < len; ++i) {

We prefer to spell out variables like length.

You need to add all the new UIStrings to localizedString.js

Awesome work!
Comment 8 Joseph Pecoraro 2009-07-30 08:35:53 PDT
Video of what it looks and feels like:
http://screencast.com/t/3yG1ZtoE
Comment 9 Joseph Pecoraro 2009-07-30 08:57:14 PDT
Created attachment 33785 [details]
Edit CSS Selectors (diff --binary)
Comment 10 Joseph Pecoraro 2009-07-30 08:59:35 PDT
(In reply to comment #7)
> (From update of attachment 33783 [details])
> > +        for (var i = 0, len = parentRules.length; i < len; ++i) {
> > +        for (var i = 0, len = nodes.length; i < len; ++i) {
> 
> We prefer to spell out variables like length.

Done. Loop condition is now array.length


> You need to add all the new UIStrings to localizedString.js

Done. In the non-pretty patch there is the Git Binary diff.  I added 3 UI Strings, including removing the old "Inline Style Attribute" in favor of the shorter "Style Attribute".
Comment 11 Adam Barth 2009-08-02 00:42:04 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/inspector/front-end/ElementsPanel.js
	M	WebCore/inspector/front-end/StylesSidebarPane.js
	M	WebCore/inspector/front-end/inspector.css
Committed r46690
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/StylesSidebarPane.js
	M	WebCore/inspector/front-end/ElementsPanel.js
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/English.lproj/localizedStrings.js
r46690 = a7c19c01c9c9ce43b1e82f9a827ca349037f5b7d (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46690
Comment 12 ryankshaw+webkitbugzilla 2009-08-11 15:05:16 PDT
I just got the latest webkit nightly and tried it out.

if you double click to create a new style rule, 
add something like color:red; ,
It makes that thing red.

now, reload the page, inspect the same thing, double click to add a new style rule that is the same as before with color:red; and it does not change anything on the page, even though in the web inspector it thinks that it is because it crosses out the rule coming from the stylesheet.

this shows what I am talking about:
http://screencast.com/t/MsmXR6Icm
Comment 13 Timothy Hatcher 2009-08-11 15:22:03 PDT
I think what is happeneing here is that we have a reference to the style element we created for the previous page still, so we think we don't need to make a new one and just append to the old one in the previous page.

This reference should be cleared in the ElementsPanel reset method.

Can you file a new bug about this? (And close this bug.)
Comment 14 Adam Barth 2009-08-11 15:34:51 PDT
We don't need the [commit+] fake flag any more.
Comment 15 Eric Seidel (no email) 2009-08-12 15:20:33 PDT
http://trac.webkit.org/changeset/46690