Bug 81414 – Web Inspector: newly added CSS rules shouldn’t rewrite HTML content in the resources panel

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

[Nikita Vasilyev]

http://www.screenr.com/jvN8

Adding CSS rule rewrites HTML content in the resources panel. Wouldn’t it be better put it into the new item under "Stylesheets" category?

[Alexander Pavlov (apavlov)]

Created attachment 132804 [details]
Patch

[Vsevolod Vlasov]

Comment on attachment 132804 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=132804&action=review

> Source/WebCore/ChangeLog:3
> +        Web Inspector: newly added CSS rules shouldnât rewrite HTML content in the resources panel

shouldn't

[Alexander Pavlov (apavlov)]

Committed r111384: <http://trac.webkit.org/changeset/111384>

[Nikita Vasilyev]

Now newly added CSS rules don’t change HTML content, which is good, but how do I see them in resources panel? I might want to save them. I originally suggested to add 'new item under "Stylesheets" category', but you haven’t implemented it and marked the issue as RESOLVED FIXED without any comments.

[Alexander Pavlov (apavlov)]

The landed patch fixes the erroneous behavior stated in the bug summary. The behavior you suggest is not that easy to implement UI-wise, since a page can contain an arbitrary number of inline stylesheets, all of which should be displayed consistently in the Resources panel. Also, it is not wise to postpone a fix for such a conspicuous bug until a complicated solution is in place.

That said, you can track the progress of bug 62294 which attempts to implement what you want to see.

[Nikita Vasilyev]

Chrome fires the following code on adding new CSS rules. Does your patch change that?

chrome.devtools.inspectedWindow.onResourceContentCommitted.addListener(function(event) {
  event.type // 'document'
  event.url // URL of HTML page
});

[Alexander Pavlov (apavlov)]

(In reply to comment #6)
> Chrome fires the following code on adding new CSS rules. Does your patch change that?
> 
> chrome.devtools.inspectedWindow.onResourceContentCommitted.addListener(function(event) {
>   event.type // 'document'
>   event.url // URL of HTML page
> });

This extension event is (indirectly) dispatched from WebInspector.Resource.prototype.addRevision(), which is no longer called when a stylesheet with a URL corresponding to a non-stylesheet resource is changed.

Andrey, is the first part of this statement correct? AFAIU, your ExtensionServer subscribes to the WebInspector.ResourceTreeModel.EventTypes.ResourceContentCommitted event dispatched from WebInspector.Resource.prototype.addRevision().

[Nikita Vasilyev]

(In reply to comment #7)
> (In reply to comment #6)
> > Chrome fires the following code on adding new CSS rules. Does your patch change that?
> > 
> > chrome.devtools.inspectedWindow.onResourceContentCommitted.addListener(function(event) {
> >   event.type // 'document'
> >   event.url // URL of HTML page
> > });

I just checked, Chrome 19.0.1076.0 canary does not fire it anymore. It makes saving newly added CSS rules impossible, please roll it back!

[Alexander Pavlov (apavlov)]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > Chrome fires the following code on adding new CSS rules. Does your patch change that?
> > > 
> > > chrome.devtools.inspectedWindow.onResourceContentCommitted.addListener(function(event) {
> > >   event.type // 'document'
> > >   event.url // URL of HTML page
> > > });
> 
> I just checked, Chrome 19.0.1076.0 canary does not fire it anymore. It makes saving newly added CSS rules impossible, please roll it back!

So, you are fine with rewriting the main document HTML content the way it was before the fix (the onResourceContentCommitted event you describe means just that)? What do you expect to see in the Resources panel when a new rule is added?

[Nikita Vasilyev]

https://groups.google.com/forum/#!topic/google-chrome-developer-tools/-c37nQPrcv4

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #6)
> > > > Chrome fires the following code on adding new CSS rules. Does your patch change that?
> > > > 
> > > > chrome.devtools.inspectedWindow.onResourceContentCommitted.addListener(function(event) {
> > > >   event.type // 'document'
> > > >   event.url // URL of HTML page
> > > > });
> > 
> > I just checked, Chrome 19.0.1076.0 canary does not fire it anymore. It makes saving newly added CSS rules impossible, please roll it back!
> 
> So, you are fine with rewriting the main document HTML content the way it was before the fix (the onResourceContentCommitted event you describe means just that)? What do you expect to see in the Resources panel when a new rule is added?

It allowed me to catch newly added CSS rules, now it doesn’t. It does have issues https://groups.google.com/forum/#!topic/google-chrome-developer-tools/-c37nQPrcv4 but it’s better to keep it as until better substitute is found.

[Michael Lajlev]

Any idea when it gonna be fixed?

[Artiom Neganov]

I see it is "FIXED". Well, the fixness means avioding responsibility, I suspect. 
Do you every consider consequences?

This change is a disaster. It is blocking for all web devs moved their development partially or fully from their off-line apps to DevTools. 

Now watching this demolishing change I feel dismayed, and the only option now is to rollback to 18th.

Thank you!

[Pavel Feldman]

(In reply to comment #12)
> I see it is "FIXED". Well, the fixness means avioding responsibility, I suspect. 
> Do you every consider consequences?
> 
> This change is a disaster. It is blocking for all web devs moved their development partially or fully from their off-line apps to DevTools. 
> 
> Now watching this demolishing change I feel dismayed, and the only option now is to rollback to 18th.
> 
> Thank you!

Nikita, do you think you can address it on your end?

[Nikita Vasilyev]

(In reply to comment #13)
> (In reply to comment #12)
> > I see it is "FIXED". Well, the fixness means avioding responsibility, I suspect. 
> > Do you every consider consequences?
> > 
> > This change is a disaster. It is blocking for all web devs moved their development partially or fully from their off-line apps to DevTools. 
> > 
> > Now watching this demolishing change I feel dismayed, and the only option now is to rollback to 18th.
> > 
> > Thank you!
> 
> Nikita, do you think you can address it on your end?

No. onResourceContentCommitted doesn’t fire. It used to do.

[Pavel Feldman]

(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > I see it is "FIXED". Well, the fixness means avioding responsibility, I suspect. 
> > > Do you every consider consequences?
> > > 
> > > This change is a disaster. It is blocking for all web devs moved their development partially or fully from their off-line apps to DevTools. 
> > > 
> > > Now watching this demolishing change I feel dismayed, and the only option now is to rollback to 18th.
> > > 
> > > Thank you!
> > 
> > Nikita, do you think you can address it on your end?
> 
> No. onResourceContentCommitted doesn’t fire. It used to do.

So we are talking about (+) "New Style Rule" action. Users that are adding rules into the existing stylesheets via editing them are not affected?

[Nikita Vasilyev]

(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > (In reply to comment #12)
> > > > I see it is "FIXED". Well, the fixness means avioding responsibility, I suspect. 
> > > > Do you every consider consequences?
> > > > 
> > > > This change is a disaster. It is blocking for all web devs moved their development partially or fully from their off-line apps to DevTools. 
> > > > 
> > > > Now watching this demolishing change I feel dismayed, and the only option now is to rollback to 18th.
> > > > 
> > > > Thank you!
> > > 
> > > Nikita, do you think you can address it on your end?
> > 
> > No. onResourceContentCommitted doesn’t fire. It used to do.
> 
> So we are talking about (+) "New Style Rule" action. Users that are adding rules into the existing stylesheets via editing them are not affected?

Correct.

[Pavel Feldman]

I landed http://trac.webkit.org/changeset/118367 that introduces the concept of the virtual resource for the inspector stylesheet. Could you check if your extension can be tweaked to support it?

[Pavel Feldman]

(In reply to comment #17)
> I landed http://trac.webkit.org/changeset/118367 that introduces the concept of the virtual resource for the inspector stylesheet. Could you check if your extension can be tweaked to support it?

(That was for Nikita)

[Nikita Vasilyev]

(In reply to comment #17)
> I landed http://trac.webkit.org/changeset/118367 that introduces the concept of the virtual resource for the inspector stylesheet. Could you check if your extension can be tweaked to support it?

It works again https://github.com/NV/chrome-devtools-autosave/commit/627534b32edc87619e448ab882e877726663f714.
Thanks!