Closed Bug 884887 Opened 7 years ago Closed 7 years ago

Move webconsole.css into themes/shared

Categories

(DevTools :: Console, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 24

People

(Reporter: msucan, Assigned: msucan)

References

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 file, 2 obsolete files)

We currently duplicate a lot of CSS in webconsole.css for each platform. We should use browser/themes/shared/devtools/webconsole.css and have only system-specific styles in the linux/osx/windows folders.
Attached patch proposed patch (obsolete) — Splinter Review
This patch adds a new webconsole.inc.css in browser/themes/shared/devtools. This new file holds all of the common style rules. The webconsole.css files from each themes/<platform>/devtools folder now only contain minimal platform-specific styling rules.
Attachment #764827 - Flags: review?(rcampbell)
Attached patch updated patch (obsolete) — Splinter Review
Optimizer pointed out we have some obsolete webconsole-related styles in browser.css. Removed them here.

Should I ask for a browser peer review for removal only changes? I think this is safe enough.
Attachment #764827 - Attachment is obsolete: true
Attachment #764827 - Flags: review?(rcampbell)
Attachment #765295 - Flags: review?(rcampbell)
Blocks: 880868
(In reply to Mihai Sucan [:msucan] from comment #2)
> Created attachment 765295 [details] [diff] [review]
> updated patch
> 
> Optimizer pointed out we have some obsolete webconsole-related styles in
> browser.css. Removed them here.
> 
> Should I ask for a browser peer review for removal only changes? I think
> this is safe enough.

should be fine, but we can ask.
Please do not remove those web console related styles from browser.css in this patch. I have already removed them in bug 873848. Its already r+'ed form browser peer and ready to land. You can rebase your patch on top of it.
Comment on attachment 765295 [details] [diff] [review]
updated patch

Review of attachment 765295 [details] [diff] [review]:
-----------------------------------------------------------------

Need to include aero theme for windows. Otherwise r+.

I doubt this needs review by a browser peer.

::: browser/themes/linux/devtools/webconsole.css
@@ -268,5 @@
> -}
> -
> -.webconsole-msg-security.webconsole-msg-warn {
> -  -moz-image-region: rect(32px, 24px, 40px, 16px);
> -}

delightful red.

::: browser/themes/windows/jar.mn
@@ +175,5 @@
>          skin/classic/browser/devtools/breadcrumbs/ltr-end-pressed.png              (devtools/breadcrumbs/ltr-end-pressed.png)
>          skin/classic/browser/devtools/breadcrumbs/ltr-end-selected-pressed.png     (devtools/breadcrumbs/ltr-end-selected-pressed.png)
>          skin/classic/browser/devtools/breadcrumbs/ltr-end-selected.png             (devtools/breadcrumbs/ltr-end-selected.png)
>          skin/classic/browser/devtools/breadcrumbs/ltr-end.png                      (devtools/breadcrumbs/ltr-end.png)
>          skin/classic/browser/devtools/breadcrumbs/ltr-middle-pressed.png           (devtools/breadcrumbs/ltr-middle-pressed.png)

missed the skin/classic/aero entry.
Attachment #765295 - Flags: review?(rcampbell) → review+
Comment on attachment 765295 [details] [diff] [review]
updated patch

The removals from browser.css look fine to me.
Attachment #765295 - Flags: feedback+
Rob, thanks for the reminder about aero.

Girish, I'll submit a rebased patch on top of yours.
Depends on: 873848
Attached patch rebased patchSplinter Review
Attachment #765295 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/2fd3d3b584d8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.