WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
42379
Web Inspector: Rename method for CSS rule source range retrieval and fix return object format
https://bugs.webkit.org/show_bug.cgi?id=42379
Summary
Web Inspector: Rename method for CSS rule source range retrieval and fix retu...
Alexander Pavlov (apavlov)
Reported
2010-07-15 08:47:36 PDT
getRuleRangesForStyleSheetId looks too long compared to other InspectorBackend methods. Also, it is more convenient to use styleId rather than ruleId as the key in the response object.
Attachments
[PATCH] Suggested solution
(13.25 KB, patch)
2010-07-15 09:46 PDT
,
Alexander Pavlov (apavlov)
pfeldman
: review-
Details
Formatted Diff
Diff
[PATCH] Comment addressed
(13.73 KB, patch)
2010-07-20 02:14 PDT
,
Alexander Pavlov (apavlov)
no flags
Details
Formatted Diff
Diff
[PATCH] Revert a removed blank line
(13.54 KB, patch)
2010-07-20 02:46 PDT
,
Alexander Pavlov (apavlov)
pfeldman
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Alexander Pavlov (apavlov)
Comment 1
2010-07-15 09:46:22 PDT
Created
attachment 61676
[details]
[PATCH] Suggested solution
Pavel Feldman
Comment 2
2010-07-18 12:25:46 PDT
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!
Alexander Pavlov (apavlov)
Comment 3
2010-07-18 14:22:50 PDT
(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.
Alexander Pavlov (apavlov)
Comment 4
2010-07-20 02:14:42 PDT
Created
attachment 62045
[details]
[PATCH] Comment addressed
Alexander Pavlov (apavlov)
Comment 5
2010-07-20 02:46:18 PDT
Created
attachment 62046
[details]
[PATCH] Revert a removed blank line
Alexander Pavlov (apavlov)
Comment 6
2010-07-22 06:27:16 PDT
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
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug