Summary: | Web Inspector: Enable switching between revisions of stylesheets | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexander Pavlov (apavlov) <apavlov> | ||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Alexander Pavlov (apavlov) <apavlov> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | 50241 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Alexander Pavlov (apavlov)
2010-11-30 05:22:29 PST
Created attachment 75131 [details]
[PATCH] Suggested solution
Comment on attachment 75131 [details] [PATCH] Suggested solution View in context: https://bugs.webkit.org/attachment.cgi?id=75131&action=review > WebCore/inspector/front-end/CSSStyleModel.js:168 > + function callback(styleSheetId, href, content) Just use styleSheetId from the outer scope without passing it as additional explicit parameter, this way you would avoid callback.bind call below. > WebCore/inspector/front-end/CSSStyleModel.js:221 > + return payload ? new WebInspector.CSSStyleDeclaration(payload) : null; The contract should be that the caller checks for null payload because otherwise we have two null checks instead of one, first one in the parse method and the second one at the call site where we check for null parsed value. > WebCore/inspector/front-end/CSSStyleModel.js:369 > + return payload ? new WebInspector.CSSRule(payload) : null; ditto > WebCore/inspector/front-end/CSSStyleModel.js:410 > + if (!payload) ditto > WebCore/inspector/front-end/CSSStyleModel.js:530 > + return payload ? new WebInspector.CSSStyleSheet(payload) : null; ditto Created attachment 75135 [details]
[PATCH] Comment addressed
Comment on attachment 75135 [details] [PATCH] Comment addressed View in context: https://bugs.webkit.org/attachment.cgi?id=75135&action=review > WebCore/inspector/front-end/CSSStyleModel.js:-538 > - function callback(styleSheetPayload) Please revert this. (In reply to comment #4) > (From update of attachment 75135 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75135&action=review > > > WebCore/inspector/front-end/CSSStyleModel.js:-538 > > - function callback(styleSheetPayload) > > Please revert this. The code in question was wrong. setStyleSheetText2() does not return a stylesheet payload. Instead, it returns the stylesheet "href" and the new textual contents. Yet, I'm adding back the missing userCallback invocation. (In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 75135 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=75135&action=review > > > > > WebCore/inspector/front-end/CSSStyleModel.js:-538 > > > - function callback(styleSheetPayload) > > > > Please revert this. > > The code in question was wrong. setStyleSheetText2() does not return a stylesheet payload. Instead, it returns the stylesheet "href" and the new textual contents. Yet, I'm adding back the missing userCallback invocation. Sorry for the confusion. setStyleSheetText2() does not return anything. It's getStyleSheetText2() that returns the href and stylesheet contents. Created attachment 75140 [details]
[PATCH] Reverted userCallback invocation
Comment on attachment 75140 [details] [PATCH] Reverted userCallback invocation View in context: https://bugs.webkit.org/attachment.cgi?id=75140&action=review > WebCore/inspector/front-end/CSSStyleModel.js:173 > + if (userCallback) Let's get rid of the unused callbacks. We can add them later once we need them. > WebCore/inspector/front-end/CSSStyleModel.js:549 > setText: function(newText, userCallback) This method is not used, consider removing it completely. Created attachment 75144 [details]
[PATCH] Removed unused method and user callback
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/English.lproj/localizedStrings.js M WebCore/inspector/InspectorStyleSheet.cpp M WebCore/inspector/front-end/CSSStyleModel.js M WebCore/inspector/front-end/ResourcesPanel.js Committed r72914 Comment on attachment 75144 [details]
[PATCH] Removed unused method and user callback
I have a much better change that is using universal rollback callback and works both for styles and javascript. It does not introduce dependency from resource panel to style sheet model + does not introduce weird .styleSheetId property. Anyways, I did not land it since setStyleSeetText was not working. Is it fixed now?
Created attachment 75147 [details]
[PATCH] My version of the patch.
Comment on attachment 75147 [details]
[PATCH] My version of the patch.
You need to add:
m_pageStyleSheet->styleSheetChanged();
after
m_pageStyleSheet->parseString(text, m_pageStyleSheet->useStrictParsing());
in
InspectorStyleSheet::reparseStyleSheet(const String& text).
|