DevTools themes - move toolbar and tab styles into shared CSS file

RESOLVED FIXED in Firefox 28

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

(Blocks 1 bug)

unspecified
Firefox 28
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 3 obsolete attachments)

Right now these are spread across three files.  With the upcoming need to theme these with light and dark themes, they are going to need access to theme colors, so it makes sense to have a single file imported from the theme files.  We will need to make sure that they look alright across operating systems, as there are some small differences between the three files right now:

* https://mxr.mozilla.org/mozilla-central/source/browser/themes/linux/devtools/common.css
* https://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/devtools/common.css
* https://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/devtools/common.css
FYI, I'm planning on basing the shared toolbars CSS file off of the linux common.css file, bringing in any changes to make it look the same on OSX as it currently does.  I'll push to try to pull down a copy on Windows and make sure it still looks alright there.
Posted patch shared-toolbar-styles.patch (obsolete) — Splinter Review
First version of shared toolbar styles
Status: NEW → ASSIGNED
What about /themes/*/devtools/toolbox.css ?
(In reply to Paul Rouget [:paul] from comment #3)
> What about /themes/*/devtools/toolbox.css ?

Good catch, I am moving these now.  There is something a little funny about these files - they contain both toolbox level stuff, and options panel stuff.  I'm going to move all of the toolbox stuff into this new file, then we have a few options for what to do with the options panel stuff:

1) Keep the few option panel rules in */devtools/toolbox.css and rename to options-panel.css and only include it in the options panel
2) Delete */devtools/toolbox.css and dump all of the leftover option panel rules into devtools/framework/toolbox.css (and rename that shared file to options-panel).
3) Move everything as-is into the new shared file, and have options panel specifics inside of the theme file essentially.
Posted patch shared-toolbar-styles.patch (obsolete) — Splinter Review
Including */toolbox.css and relevant devtools/framework/toolbox.css changes.  Renamed devtools/framework/toolbox.css to devtools/framework/options-panel.css and migrated all options-panel specific styling that was in */toolbox.css here.  Pushed to try so I can have a look on different operating systems: https://tbpl.mozilla.org/?tree=Try&rev=257fe78ce68f
Attachment #8336967 - Attachment is obsolete: true
Posted patch shared-toolbar-styles.patch (obsolete) — Splinter Review
OK, I've fixed most of the Windows issues from the previous pushes.  I have one more fix to double check on its way up here: https://tbpl.mozilla.org/?tree=Try&rev=fd6b9838c16c.

This patch is large, but it isn't doing too much.  Here is what it is doing:

* Move toolbar styles from themes/*/devtools/common.css into themes/shared/devtools/toolbars.inc.css
* Rename devtools/framework/toolbox.css to devtools/framework/options-panel.css and put all options panel styles here.
* Move toolbox styles from devtools/framework/toolbox.css and themes/*/devtools/toolbox.css into themes/shared/devtools/toolbars.inc.css
* Allow preprocessing on light and dark theme files, so that they can include toolbars.inc.css

I based the shared file mostly off of the Linux files, but copied over styles where they appeared to be needed for OSX and Windows.  We should make sure everything looks OK in each of the environments.
Attachment #8337848 - Attachment is obsolete: true
Attachment #8338493 - Flags: review?(paul)
Comment on attachment 8338493 [details] [diff] [review]
shared-toolbar-styles.patch

This is refreshing :)

- indentation problem in a jar.mn
- resizer style has been lost (resizer on top of the toolbox is white)
- browser/themes/*/devtools/common.css <- no need to keep these files. You can use %ifdef (see in common.inc.css)
- there are still some option-panel specific code in toolbar.inc.css
Attachment #8338493 - Flags: review?(paul) → review-
> - indentation problem in a jar.mn

Fixed.

> - resizer style has been lost (resizer on top of the toolbox is white)

Fixed - I didn't realize that the splitter styles needed to be in common.css (since this is actually being included in browser.xul).

> - browser/themes/*/devtools/common.css <- no need to keep these files. You
> can use %ifdef (see in common.inc.css)

Removed and added the ifdefs inside of common.inc.css (now just named common.css).

> - there are still some option-panel specific code in toolbar.inc.css

That was duplicated code - just removed it.
Attachment #8338493 - Attachment is obsolete: true
Attachment #8341091 - Flags: review?(paul)
Attachment #8341091 - Flags: review?(paul) → review+
https://hg.mozilla.org/mozilla-central/rev/71fcb255a8cf
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
Depends on: 947346
Depends on: 950483
Whiteboard: [qa-]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.