Last Comment Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed
: Allow the source code editor of Scratchpad & StyleEditor to be themed
Status: RESOLVED FIXED
[sourceeditor][orion][good first bug]...
: dev-doc-needed
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 13
Assigned To: VD
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-26 02:36 PST by Alfred Kayser
Modified: 2012-02-09 12:43 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed (11.36 KB, patch)
2012-01-29 03:18 PST, VD
no flags Details | Diff | Splinter Review
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed (20.07 KB, patch)
2012-01-29 03:59 PST, VD
mihai.sucan: review+
dao+bmo: review-
Details | Diff | Splinter Review
Patch for Bug 721324 (68.56 KB, patch)
2012-01-30 05:15 PST, VD
no flags Details | Diff | Splinter Review
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed (23.66 KB, patch)
2012-01-30 05:44 PST, VD
no flags Details | Diff | Splinter Review
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed (23.14 KB, patch)
2012-01-30 05:55 PST, VD
no flags Details | Diff | Splinter Review
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed (23.07 KB, patch)
2012-01-30 06:03 PST, VD
mihai.sucan: review+
dao+bmo: review+
Details | Diff | Splinter Review
Fresh patch after a hg copy of mozilla.css - no change to orion.css (8.79 KB, patch)
2012-01-30 08:08 PST, VD
no flags Details | Diff | Splinter Review
[in-fx-team] minor update (20.75 KB, patch)
2012-02-06 11:53 PST, Mihai Sucan [:msucan]
no flags Details | Diff | Splinter Review

Description Alfred Kayser 2012-01-26 02:36:15 PST
The source editor (orion) currently only includes 'chrome://browser/content/orion-mozilla.css'. This means that third party themes, such as Walnut and Nautipolis cannot style the style editor nor scratchpad.

Please add a way to include a 'chrome://browser/skin/orion-mozilla.css'.
Possibly with the default theme with an import to the content version.
@import "chrome://browser/content/orion-mozilla.css"
Comment 1 Mihai Sucan [:msucan] 2012-01-26 03:27:38 PST
Sounds like a good idea! Thanks Alfred!
Comment 2 VD 2012-01-28 21:08:11 PST
HI! I would like to work on this bug! But this is my first bug! Could you guide me on how to proceed? I have pulled the source code from the repository!
Comment 3 Mihai Sucan [:msucan] 2012-01-29 02:05:33 PST
(In reply to VD from comment #2)
> HI! I would like to work on this bug! But this is my first bug! Could you
> guide me on how to proceed? I have pulled the source code from the
> repository!

Hello VD! Thanks for your interest to help us with this bug! I am glad to help you make the Source Editor themeable.

We have taken the discussion to IRC and VD is already working on a patch! Looking forward to his patch!
Comment 4 VD 2012-01-29 03:18:03 PST
Created attachment 592479 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed
Comment 5 VD 2012-01-29 03:18:44 PST
hope this helps :)
Comment 6 VD 2012-01-29 03:59:59 PST
Created attachment 592482 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed
Comment 7 Mihai Sucan [:msucan] 2012-01-29 04:55:57 PST
Comment on attachment 592482 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed

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

Path looks good. Thank you very much for your contribution VD! Awesome!

Dão: should we keep the content stylesheet? I think we can work only with the theme CSS.

::: browser/devtools/sourceeditor/orion/mozilla.css
@@ +5,4 @@
>  }
>  
>  .view {
> + }

Nit: you add an unneeded space here.
Comment 8 Dão Gottwald [:dao] 2012-01-30 02:45:20 PST
Comment on attachment 592482 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed

>--- a/browser/devtools/sourceeditor/orion/mozilla.css	Mon Jan 23 23:06:58 2012 +0100
>+++ b/browser/devtools/sourceeditor/orion/mozilla.css	Sun Jan 29 19:54:09 2012 +0800
>@@ -1,129 +1,86 @@
> /* Any copyright is dedicated to the Public Domain.
>    http://creativecommons.org/publicdomain/zero/1.0/ */
> 
>-.viewContainer {
>-  background: #cddae5; /* This will be seen as the continuation of the ruler */
>-  font-family: monospace;
>-  font-size: inherit; /* inherit browser's default monospace font size */
>+.viewContainer {  
> }

Please remove any empty rules.

Please use 'hg copy' to move the mozilla.css contents to the new files (before modifying mozilla.css).

>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/browser/themes/winstripe/devtools/orion-mozilla.css	Sun Jan 29 19:54:09 2012 +0800
>@@ -0,0 +1,127 @@
>+/* Any copyright is dedicated to the Public Domain.
>+   http://creativecommons.org/publicdomain/zero/1.0/ */

Should use the MPL here. http://www.mozilla.org/MPL/headers/

>--- a/browser/themes/winstripe/jar.mn	Mon Jan 23 23:06:58 2012 +0100
>+++ b/browser/themes/winstripe/jar.mn	Sun Jan 29 19:54:09 2012 +0800
>@@ -105,16 +105,17 @@ browser.jar:
>         skin/classic/browser/tabview/tabview.png                    (tabview/tabview.png)
>         skin/classic/browser/tabview/tabview-inverted.png           (tabview/tabview-inverted.png)
>         skin/classic/browser/tabview/tabview.css                    (tabview/tabview.css)
>         skin/classic/browser/devtools/common.css                    (devtools/common.css)
>         skin/classic/browser/devtools/arrows.png                    (devtools/arrows.png)
>         skin/classic/browser/devtools/goto-mdn.png                  (devtools/goto-mdn.png)
>         skin/classic/browser/devtools/csshtmltree.css               (devtools/csshtmltree.css)
>         skin/classic/browser/devtools/gcli.css                      (devtools/gcli.css)
>+	skin/classic/browser/devtools/orion-mozilla.css                    (devtools/orion-mozilla.css)

nit: indentation is off.

Please name the new files orion.css rather than orion-mozilla.css.
Comment 9 VD 2012-01-30 05:15:30 PST
Created attachment 592665 [details] [diff] [review]
Patch for Bug 721324
Comment 10 VD 2012-01-30 05:44:06 PST
Created attachment 592671 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed
Comment 11 VD 2012-01-30 05:55:14 PST
Created attachment 592677 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed
Comment 12 VD 2012-01-30 06:03:07 PST
Created attachment 592685 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed
Comment 13 Dão Gottwald [:dao] 2012-01-30 06:19:50 PST
Comment on attachment 592685 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed

>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/browser/themes/winstripe/devtools/orion.css	Mon Jan 30 21:42:48 2012 +0800

I expected 'hg copy' to create this file based on browser/devtools/sourceeditor/orion/mozilla.css rather than /dev/null. Did this not work?
Comment 14 VD 2012-01-30 07:57:25 PST
Hi! I had initially done a hg add and then removed those files and did a hg copy of mozilla.css. Not sure why its still /dev/null!
Comment 15 VD 2012-01-30 08:08:53 PST
Created attachment 592725 [details] [diff] [review]
Fresh patch after a hg copy of mozilla.css - no change to orion.css
Comment 16 Mihai Sucan [:msucan] 2012-02-01 07:26:13 PST
Comment on attachment 592685 [details] [diff] [review]
Patch for Bug 721324 - Allow the source code editor of Scratchpad & StyleEditor to be themed

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

Patch looks good for me! Thanks VD for your work and sorry for all the trouble!

Dão: the hg copy trick does not seem to do what we would expect - could be because the copy happens to three locations. Can you please r+ this patch from our contributor? I appreciate his work and I believe that this patch should land, unless there are other problems with it. Thank you!
Comment 17 Mihai Sucan [:msucan] 2012-02-06 11:53:37 PST
Created attachment 594765 [details] [diff] [review]
[in-fx-team] minor update

VD: I removed orion-mozilla.css entirely from browser/devtools - not much point in keeping it.

. and I landed your patch! See:
https://hg.mozilla.org/integration/fx-team/rev/e31fff76aeff

Thank you very much for your contribution! Much appreciated! Keep up the good work!
Comment 18 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-02-07 11:21:33 PST
https://hg.mozilla.org/mozilla-central/rev/e31fff76aeff
Comment 19 Mihai Sucan [:msucan] 2012-02-09 12:43:02 PST
Please document the new themeable Source Editor (Orion) CSS files in Firefox 13. Thank you!

Note You need to log in before you can comment on or make changes to this bug.