Bug 148502 - Web Inspector: Separate creating a style sheet from adding a new rule in the protocol
Summary: Web Inspector: Separate creating a style sheet from adding a new rule in the ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-08-26 17:13 PDT by Joseph Pecoraro
Modified: 2015-08-28 10:59 PDT (History)
8 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (31.71 KB, patch)
2015-08-26 17:52 PDT, Joseph Pecoraro
joepeck: review-
Details | Formatted Diff | Diff
[PATCH] Proposed Fix (42.60 KB, patch)
2015-08-26 23:01 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-08-26 17:13:42 PDT
* SUMMARY
Separate creating a style sheet from adding a new rule.

Currently the only way to create a new style from the inspector is by calling "CSSAgent.addRule" this is opaque to the frontend. Also, CSSAgent.addRule takes a contextNode to determine the frame/document to create the new style sheet. Extract createStyleSheet(frameId) => styleSheetId and convert addRule(contextNode, ...) to addRule(styleSheetId, ...).

* NOTES
- Blink has done the same here.
- This will be useful if we want a way to view/edit all inspector added styles for a page, e.g. everything added via the style sidebar
- This will be useful if we want the frontend to create arbitrary stylesheets for a page, e.g. in page building
- Also, the frontend can use the new addRule to add rules to an existing stylesheet instead of the inspector stylesheet (might be useful for diffs / editing workflows)
Comment 1 Radar WebKit Bug Importer 2015-08-26 17:14:29 PDT
<rdar://problem/22448323>
Comment 2 Joseph Pecoraro 2015-08-26 17:52:01 PDT
Created attachment 260011 [details]
[PATCH] Proposed Fix

An open question here would be, should we allow creating more than one StyleSheet per-Frame?

This patch says no, the backend will only create one. We could change that.
Comment 3 Timothy Hatcher 2015-08-26 18:04:30 PDT
One question I have been wondering is how a playground like feature could use this. It might best to have that create a separate style sheet. But it could also append rules to the existing inspector sheet. It would be easier if it didn't have to worry about mixing with other edits, so deprecate sheets might be more desirable and easier for us to manage.
Comment 4 Joseph Pecoraro 2015-08-26 22:20:41 PDT
(In reply to comment #3)
> One question I have been wondering is how a playground like feature could
> use this. It might best to have that create a separate style sheet. But it
> could also append rules to the existing inspector sheet. It would be easier
> if it didn't have to worry about mixing with other edits, so deprecate
> sheets might be more desirable and easier for us to manage.

Yeah, I'm going to put up a new patch that makes:

  - CSS.createStyleSheet(frameId) always creates a new style sheet for that frame
  - CSSManager.preferredInspectorStyleSheetForFrame(frame) returns one, preferred, style sheet for "Web Inspector" added styles.
Comment 5 Joseph Pecoraro 2015-08-26 23:01:37 PDT
Created attachment 260036 [details]
[PATCH] Proposed Fix

This makes a lot more sense!
Comment 6 WebKit Commit Bot 2015-08-28 10:59:46 PDT
Comment on attachment 260036 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 260036

Committed r189104: <http://trac.webkit.org/changeset/189104>
Comment 7 WebKit Commit Bot 2015-08-28 10:59:49 PDT
All reviewed patches have been landed.  Closing bug.