Bug 111173 - Web Inspector: Move sidebar-specific styles to a separate file
Summary: Web Inspector: Move sidebar-specific styles to a separate file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Vladislav Kaznacheev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-01 05:57 PST by Vladislav Kaznacheev
Modified: 2013-03-06 07:25 PST (History)
11 users (show)

See Also:


Attachments
Patch (21.58 KB, patch)
2013-03-01 06:01 PST, Vladislav Kaznacheev
no flags Details | Formatted Diff | Diff
Patch (22.51 KB, patch)
2013-03-04 07:45 PST, Vladislav Kaznacheev
no flags Details | Formatted Diff | Diff
Patch (28.06 KB, patch)
2013-03-05 05:44 PST, Vladislav Kaznacheev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vladislav Kaznacheev 2013-03-01 05:57:47 PST
Create sidebarPane.css and load it on demand.
Comment 1 Vladislav Kaznacheev 2013-03-01 06:01:01 PST
Created attachment 190952 [details]
Patch
Comment 2 Pavel Feldman 2013-03-01 06:12:52 PST
Comment on attachment 190952 [details]
Patch

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

> Source/WebCore/inspector/front-end/sidebarPane.css:63
> +.sidebar-tabbed-pane .tabbed-pane-header {

SidebarPane does not know anything about tabbed-pane-header

> Source/WebCore/inspector/front-end/sidebarPane.css:131
> +.sidebar-pane-toolbar > select {

All of these are elementsPanel specific

> Source/WebCore/inspector/front-end/sidebarPane.css:287
> +.event-listener-breakpoints .event-category {

These are elements panel specific.
Comment 3 Vladislav Kaznacheev 2013-03-04 07:45:28 PST
Created attachment 191244 [details]
Patch
Comment 4 Vladislav Kaznacheev 2013-03-04 07:49:02 PST
Comment on attachment 190952 [details]
Patch

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

>> Source/WebCore/inspector/front-end/sidebarPane.css:63
>> +.sidebar-tabbed-pane .tabbed-pane-header {
> 
> SidebarPane does not know anything about tabbed-pane-header

SidebarPanes does not but SidebarTabbedPane does.

>> Source/WebCore/inspector/front-end/sidebarPane.css:131
>> +.sidebar-pane-toolbar > select {
> 
> All of these are elementsPanel specific

Moved to elementsPanel.css

>> Source/WebCore/inspector/front-end/sidebarPane.css:287
>> +.event-listener-breakpoints .event-category {
> 
> These are elements panel specific.

Moved to elementsPanel.css
Comment 5 Pavel Feldman 2013-03-05 03:56:21 PST
Comment on attachment 191244 [details]
Patch

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

> Source/WebCore/inspector/front-end/elementsPanel.css:534
> +.sidebar-pane-toolbar > select:hover {

I would imagine that this file only gets styles with ".elements-" prefix otherwise, you are at risk of breaking stuff on other panels (styles are unloaded when you switch off the panel).

> Source/WebCore/inspector/front-end/scriptsPanel.css:312
> +.sidebar-pane > .body .breakpoint-condition {

Like these ones are ok - they all mention breakpoint...

> Source/WebCore/inspector/front-end/scriptsPanel.css:328
> +ol.breakpoint-list {

And what about DOM Breakpoints? They might rely upon this style. I might be that breakpoints list view might need to have its own CSS.

> Source/WebCore/inspector/front-end/sidebarPane.css:73
> +.pane-title {

Should we rename this to .sidebar-pane-title?
Comment 6 Vladislav Kaznacheev 2013-03-05 05:38:38 PST
Comment on attachment 191244 [details]
Patch

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

>> Source/WebCore/inspector/front-end/elementsPanel.css:534
>> +.sidebar-pane-toolbar > select:hover {
> 
> I would imagine that this file only gets styles with ".elements-" prefix otherwise, you are at risk of breaking stuff on other panels (styles are unloaded when you switch off the panel).

Prefixed all these rules with .panel.elements

>> Source/WebCore/inspector/front-end/scriptsPanel.css:312
>> +.sidebar-pane > .body .breakpoint-condition {
> 
> Like these ones are ok - they all mention breakpoint...

Thanks, I guess.

>> Source/WebCore/inspector/front-end/scriptsPanel.css:328
>> +ol.breakpoint-list {
> 
> And what about DOM Breakpoints? They might rely upon this style. I might be that breakpoints list view might need to have its own CSS.

You caught me red-handed. Created breakpointsList.css.

>> Source/WebCore/inspector/front-end/sidebarPane.css:73
>> +.pane-title {
> 
> Should we rename this to .sidebar-pane-title?

Done
Comment 7 Vladislav Kaznacheev 2013-03-05 05:44:33 PST
Created attachment 191475 [details]
Patch
Comment 8 Build Bot 2013-03-05 16:42:29 PST
Comment on attachment 191475 [details]
Patch

Attachment 191475 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17015041

New failing tests:
editing/selection/selection-invalid-offset.html
Comment 9 WebKit Review Bot 2013-03-06 07:25:19 PST
Comment on attachment 191475 [details]
Patch

Clearing flags on attachment: 191475

Committed r144927: <http://trac.webkit.org/changeset/144927>
Comment 10 WebKit Review Bot 2013-03-06 07:25:23 PST
All reviewed patches have been landed.  Closing bug.