Summary: | Web Inspector: Rename method for CSS rule source range retrieval and fix return object format | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexander Pavlov (apavlov) <apavlov> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Alexander Pavlov (apavlov) <apavlov> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bweinstein, commit-queue, joepeck, keishi, pfeldman, pmuellr, rik, timothy, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Alexander Pavlov (apavlov)
2010-07-15 08:47:36 PDT
Created attachment 61676 [details]
[PATCH] Suggested solution
Comment on attachment 61676 [details]
[PATCH] Suggested solution
WebCore/inspector/InspectorCSSStore.cpp:141
+ return result;
Nit: I'd suggest not using result here and moving declaration to where it is used (bottom of this method).
WebCore/inspector/InspectorCSSStore.cpp:149
+ result.set(bindStyle(rule->style()), offsetVector->at(ruleIndex));
I think binding rule makes more sense. Why did this change? r- here unless you explain please!
(In reply to comment #2) > (From update of attachment 61676 [details]) > WebCore/inspector/InspectorCSSStore.cpp:141 > + return result; > Nit: I'd suggest not using result here and moving declaration to where it is used (bottom of this method). Will do. > WebCore/inspector/InspectorCSSStore.cpp:149 > + result.set(bindStyle(rule->style()), offsetVector->at(ruleIndex)); > I think binding rule makes more sense. Why did this change? r- here unless you explain please! It is quite logical to bind rules instead of styles. However: (a) Styles sidebar (and my new code) uses style bindings (rather than rule bindings) returned from InspectorDOMAgent::applyStyleText: m_frontend->didApplyStyleText(callId, true, buildObjectForStyle(style, true), toArray(changedProperties)); I will need another internal map to handle rule bindings if they are used in place of style bindings. (b) I can see this method (perhaps after getting a better name and a slight refactoring) return positions for rules in external CSS, inline CSS, and style descriptions as values of the "style" element attributes. In the latter case, there are no rules, only styles. It doesn't seem logical to return both bound rules and bound styles in similar cases. Created attachment 62045 [details]
[PATCH] Comment addressed
Created attachment 62046 [details]
[PATCH] Revert a removed blank line
Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog M LayoutTests/inspector/styles-source-offsets.html M WebCore/ChangeLog M WebCore/inspector/Inspector.idl M WebCore/inspector/InspectorBackend.cpp M WebCore/inspector/InspectorBackend.h M WebCore/inspector/InspectorBackend.idl M WebCore/inspector/InspectorCSSStore.cpp M WebCore/inspector/InspectorCSSStore.h M WebCore/inspector/InspectorDOMAgent.cpp M WebCore/inspector/InspectorDOMAgent.h M WebCore/inspector/front-end/DOMAgent.js M WebKit/chromium/ChangeLog M WebKit/chromium/src/js/InspectorControllerImpl.js Committed r63886 |