Last Comment Bug 588655 - When in private browsing mode, make the Firefox button purple
: When in private browsing mode, make the Firefox button purple
Status: RESOLVED FIXED
: dev-doc-complete, ux-mode-error
Product: Firefox
Classification: Client Software
Component: Private Browsing (show other bugs)
: Trunk
: All Windows 7
: -- normal with 3 votes (vote)
: Firefox 4.0b7
Assigned To: :Ehsan Akhgari
:
:
Mentors:
: 575289 605026 (view as bug list)
Depends on: 607482
Blocks: 588242
  Show dependency treegraph
 
Reported: 2010-08-18 18:12 PDT by Alex Faaborg [:faaborg] (Firefox UX)
Modified: 2014-04-29 23:00 PDT (History)
23 users (show)
ehsan: in‑testsuite+
mozillamarcia.knous: in‑litmus+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
final+


Attachments
Purple Firefox Button in PBM - 01 (3.84 KB, patch)
2010-09-07 21:57 PDT, Stephen Horlander [:shorlander]
no flags Details | Diff | Splinter Review
Screenshot of Purple Firefox Button (210.61 KB, image/png)
2010-09-07 21:59 PDT, Stephen Horlander [:shorlander]
no flags Details
Purple Firefox Button in PBM - 02 (3.84 KB, patch)
2010-09-12 15:32 PDT, Stephen Horlander [:shorlander]
ehsan: review-
Details | Diff | Splinter Review
Part 1: add a privatebrowsingmode=permanent attribute to browser.xul's doc root (3.40 KB, patch)
2010-09-13 14:31 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 2: actually make the button purple (4.16 KB, patch)
2010-09-13 14:40 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 2: actually make the button purple (2.91 KB, patch)
2010-09-13 15:29 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 1: add a privatebrowsingmode=permanent attribute to browser.xul's doc root (4.63 KB, patch)
2010-09-14 12:38 PDT, :Ehsan Akhgari
dao+bmo: review-
Details | Diff | Splinter Review
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root (5.47 KB, patch)
2010-09-17 13:40 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 2: actually make the button purple (4.06 KB, patch)
2010-09-17 13:50 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 2: actually make the button purple (4.02 KB, patch)
2010-09-21 12:25 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root (5.59 KB, patch)
2010-09-21 15:53 PDT, :Ehsan Akhgari
dao+bmo: review+
Details | Diff | Splinter Review
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root (5.26 KB, patch)
2010-09-22 10:10 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root (5.05 KB, patch)
2010-09-22 14:24 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 2: actually make the button purple (4.07 KB, patch)
2010-10-20 16:15 PDT, :Ehsan Akhgari
no flags Details | Diff | Splinter Review
Part 2: actually make the button purple (2.82 KB, patch)
2010-10-25 09:06 PDT, :Ehsan Akhgari
dao+bmo: review+
Details | Diff | Splinter Review

Description Alex Faaborg [:faaborg] (Firefox UX) 2010-08-18 18:12:17 PDT
As an ambient and peripheral cue that the user is in private browsing mode, we should adjust the color of the Firefox button on Windows 7 and Vista from orange to purple.  (purple is used on the masquerade mask in the content area message, and doesn't have any other semantic meaning in the product other than privacy).

Additionally we should alter the text of the button to "Firefox (Private)"

Any other modifications to the theme (like darkening the toolbars) will be done in addition to this change, but this is the minimum change for indicating the state of the browser.

A specific non-goal of this bug is over the shoulder screen privacy.  We are assuming that if you are using private browsing mode, you are probably not in a situation where other people can view your computer monitor (since the contents of content area are being shown just as much as the firefox button).

We are however, very interested in giving users a reminder that Firefox is not storing any information about their activities on the Web, and we are interested in significantly reducing the number of mode errors that users are currently experiencing (staying in Firefox for long periods of time because they forget to exit).

(listing this under content area notifications, which isn't entirely accurate since it persists after we show about:privatebrowsing, but I didn't really have any other place to put it)
Comment 1 Alex Faaborg [:faaborg] (Firefox UX) 2010-08-18 18:15:21 PDT
Nominating blocking since there is no other indicator that you are in private browsing mode on Windows Vista/7.  The change should be pretty minimal since we have a css selector for private browsing.
Comment 2 Peter Henkel [:Terepin] 2010-08-19 03:19:39 PDT
If i may request: On old PB mockup there was PB icon as indicator. However I suggest using it not only as a indicator but as off switch as well, that could be placed inside FXB. I tried this "feature" in StrataBuddy and I can say it is very comfortable.
Comment 3 Mike Beltzner [:beltzner, not reading bugmail] 2010-08-30 21:36:15 PDT
Ehsan: can you take this?
Comment 4 :Ehsan Akhgari 2010-08-31 08:00:39 PDT
(In reply to comment #3)
> Ehsan: can you take this?

Sure.
Comment 5 :Ehsan Akhgari 2010-08-31 08:33:42 PDT
I need to know which color to use as the purple background.  The current orange color we use is rgb(228,120,14).
Comment 6 Stephen Horlander [:shorlander] 2010-09-07 21:57:53 PDT
Created attachment 472932 [details] [diff] [review]
Purple Firefox Button in PBM - 01

Make the Firefox Button purple when in Private Browsing mode.
Comment 7 Stephen Horlander [:shorlander] 2010-09-07 21:59:14 PDT
Created attachment 472933 [details]
Screenshot of Purple Firefox Button
Comment 8 Dão Gottwald [:dao] 2010-09-10 08:08:45 PDT
Comment on attachment 472932 [details] [diff] [review]
Purple Firefox Button in PBM - 01

>diff --git a/browser/themes/winstripe/browser/browser-aero.css b/browser/themes/winstripe/browser/browser-aero.css
>--- a/browser/themes/winstripe/browser/browser-aero.css
>+++ b/browser/themes/winstripe/browser/browser-aero.css
>@@ -25,16 +25,22 @@
>     border-top: none;
>     -moz-border-left-colors: rgba(255,255,255,.5) rgba(83,42,6,.9);
>     -moz-border-bottom-colors: rgba(255,255,255,.5) rgba(83,42,6,.9);
>     -moz-border-right-colors: rgba(255,255,255,.5) rgba(83,42,6,.9);
>     -moz-box-shadow: 0 1px 0 rgba(255,255,255,.25) inset,
>                      0 0 2px 1px rgba(255,255,255,.25) inset;
>   }
> 
>+  [browsingmode=private]  #appmenu-button {

Use #main-window[browsingmode=private]

Do we still need some other, more accessible feedback?
Comment 9 Stephen Horlander [:shorlander] 2010-09-10 08:13:47 PDT
(In reply to comment #8)
> Do we still need some other, more accessible feedback?

It would be nice to tag the button with text in some way. Maybe "(Private) Firefox". Or maybe "<maskicon> Firefox". Not sure if we have separate bugs for those and any kind of text would have to beat string freeze.
Comment 10 :Ehsan Akhgari 2010-09-10 13:24:07 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Do we still need some other, more accessible feedback?
> 
> It would be nice to tag the button with text in some way. Maybe "(Private)
> Firefox". Or maybe "<maskicon> Firefox". Not sure if we have separate bugs for
> those and any kind of text would have to beat string freeze.

I think adding a mask icon is better, since adding the text would make the button too wide.
Comment 11 Alex Faaborg [:faaborg] (Firefox UX) 2010-09-10 15:01:07 PDT
it's really hard to do icons well on these buttons.  I think we should go with "Firefox - Private", it's wider, but that also helps the user realize that they are in a mode (and there is certainly no lack of space on the title bar).
Comment 12 Peter Henkel [:Terepin] 2010-09-11 05:25:48 PDT
The question is how to handle this with bug 571785. The only possible solution would be replacing FX icon with PB icon.
Comment 13 Stephen Horlander [:shorlander] 2010-09-11 07:07:47 PDT
(In reply to comment #12)
> The question is how to handle this with bug 571785. The only possible solution
> would be replacing FX icon with PB icon.

We could put a mask ON the Firefox ;)
Comment 14 Tan Wei Lin Jason [:Ryuji] 2010-09-11 07:49:46 PDT
Nice idea. It would look unique and it is creative.
Comment 15 Peter Henkel [:Terepin] 2010-09-11 07:57:55 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > The question is how to handle this with bug 571785. The only possible solution
> > would be replacing FX icon with PB icon.
> 
> We could put a mask ON the Firefox ;)

Even better! :)
Comment 16 Stephen Horlander [:shorlander] 2010-09-12 15:32:19 PDT
Created attachment 474585 [details] [diff] [review]
Purple Firefox Button in PBM - 02

Updated to use #main-window[browsingmode=private]
Comment 17 :Ehsan Akhgari 2010-09-12 21:12:59 PDT
Comment on attachment 474585 [details] [diff] [review]
Purple Firefox Button in PBM - 02

You should also take into account the permanent private browsing mode as well.  We don't show (Private Browsing) in the title bar for permanent private browsing mode sessions, by the same logic we shouldn't change the color of the Firefox button to purple when that mode is active.
Comment 18 Stephen Horlander [:shorlander] 2010-09-13 08:19:24 PDT
Is there a way to do that with CSS?
Comment 19 :Ehsan Akhgari 2010-09-13 14:31:27 PDT
Created attachment 474841 [details] [diff] [review]
Part 1: add a privatebrowsingmode=permanent attribute to browser.xul's doc root

This will enable us to theme based on whether the PB session is permanent or not.
Comment 20 :Ehsan Akhgari 2010-09-13 14:40:41 PDT
Created attachment 474849 [details] [diff] [review]
Part 2: actually make the button purple
Comment 21 :Ehsan Akhgari 2010-09-13 15:29:34 PDT
Created attachment 474879 [details] [diff] [review]
Part 2: actually make the button purple

With a small fix...
Comment 22 :Ehsan Akhgari 2010-09-14 12:38:13 PDT
Created attachment 475177 [details] [diff] [review]
Part 1: add a privatebrowsingmode=permanent attribute to browser.xul's doc root

Actually make sure that |docElement| is defined before using it...
Comment 23 Dão Gottwald [:dao] 2010-09-17 00:51:02 PDT
Comment on attachment 475177 [details] [diff] [review]
Part 1: add a privatebrowsingmode=permanent attribute to browser.xul's doc root

I don't see the point of "browsingmode". Seems like we want three states:

[privatebrowsingmode absent]
privatebrowsingmode=temporary
privatebrowsingmode=permanent

However, if browsingmode=normal isn't actually useful for styling, maybe privatebrowsingmode=temporary wouldn't be either. Which leaves us with:

[privatebrowsingmode absent]
privatebrowsingmode=permanent
Comment 24 Dão Gottwald [:dao] 2010-09-17 00:54:04 PDT
Oh, so you do want the color change for temporary private browsing but not otherwise. So this would leave us with this:

[privatebrowsingmode absent]
privatebrowsingmode=temporary
Comment 25 :Ehsan Akhgari 2010-09-17 08:29:32 PDT
(In reply to comment #24)
> Oh, so you do want the color change for temporary private browsing but not
> otherwise. So this would leave us with this:
> 
> [privatebrowsingmode absent]
> privatebrowsingmode=temporary

But I guess we would also need privatebrowsingmode=permanent as well, for themes which might need that for styling permanent PB sessions differently, right?  Please note that this was originally added in order to support theme styling.
Comment 26 :Ehsan Akhgari 2010-09-17 13:40:17 PDT
Created attachment 476370 [details] [diff] [review]
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root
Comment 27 :Ehsan Akhgari 2010-09-17 13:50:55 PDT
Created attachment 476373 [details] [diff] [review]
Part 2: actually make the button purple

Based on the new privatebrowsingmode attribute.
Comment 28 Dão Gottwald [:dao] 2010-09-20 04:51:30 PDT
*** Bug 575289 has been marked as a duplicate of this bug. ***
Comment 29 Luke Iliffe (Harlequin99) 2010-09-20 06:29:38 PDT
Is there a similar bug for XP when the firefox button is shown? This bug seems very win7/Vista specific.
Comment 30 Dão Gottwald [:dao] 2010-09-21 04:19:07 PDT
Comment on attachment 476370 [details] [diff] [review]
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

Ok, do we want to support special styling for privatebrowsingmode=permanent? If not, why set the attribute?
Comment 31 Dão Gottwald [:dao] 2010-09-21 04:20:57 PDT
Comment on attachment 476373 [details] [diff] [review]
Part 2: actually make the button purple

>+#appmenu-button:-moz-window-inactive,
>+#main-window[privatebrowsingmode=temporary] #appmenu-button:-moz-window-inactive {
>+appmenu-button:-moz-window-inactive {
>   background-image: none;
>   border-color: rgba(0,0,0,.4);
> }

Broken selector.
Comment 32 :Ehsan Akhgari 2010-09-21 11:42:51 PDT
(In reply to comment #30)
> Ok, do we want to support special styling for privatebrowsingmode=permanent? If
> not, why set the attribute?

We don't currently, but themes might.
Comment 33 :Ehsan Akhgari 2010-09-21 12:25:08 PDT
Created attachment 477229 [details] [diff] [review]
Part 2: actually make the button purple
Comment 34 Dão Gottwald [:dao] 2010-09-21 12:39:23 PDT
(In reply to comment #32)
> (In reply to comment #30)
> > Ok, do we want to support special styling for privatebrowsingmode=permanent? If
> > not, why set the attribute?
> 
> We don't currently, but themes might.

They might want to, but this doesn't mean it's a good idea which should be supported. I don't know why we don't have an indicator for permanent private browsing, but I assumed there would be a good reason. Is it more of an arbitrary choice?
Comment 35 :Ehsan Akhgari 2010-09-21 12:44:02 PDT
(In reply to comment #34)
> (In reply to comment #32)
> > (In reply to comment #30)
> > > Ok, do we want to support special styling for privatebrowsingmode=permanent? If
> > > not, why set the attribute?
> > 
> > We don't currently, but themes might.
> 
> They might want to, but this doesn't mean it's a good idea which should be
> supported. I don't know why we don't have an indicator for permanent private
> browsing, but I assumed there would be a good reason. Is it more of an
> arbitrary choice?

The reason is that setting Firefox to always be in private browsing mode is an explicit choice which you usually do inside the preferences window, and we assume that if you choose that setting, you're aware of the implications and do not need to be reminded about it every time you look at the Firefox window.

I'd still opt for making it possible for themes to style the permanent mode differently, but if you don't like that, I can take that out of the patch, as I don't like to keep this blocker bug blocking on that issue any more.
Comment 36 Dão Gottwald [:dao] 2010-09-21 13:51:23 PDT
Comment on attachment 476370 [details] [diff] [review]
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

>+    let docElement = document.documentElement;
>     if (this._privateBrowsingService.autoStarted) {
>       // Disable the menu item in auto-start mode
>       document.getElementById("privateBrowsingItem")
>               .setAttribute("disabled", "true");
> #ifdef MENUBAR_CAN_AUTOHIDE
>       document.getElementById("appmenu_privateBrowsing")
>               .setAttribute("disabled", "true");
> #endif
>       document.getElementById("Tools:PrivateBrowsing")
>               .setAttribute("disabled", "true");
>+      docElement.setAttribute("privatebrowsingmode", "permanent");
>     }
>     else if (window.location.href == getBrowserURL()) {
>       // Adjust the window's title
>-      let docElement = document.documentElement;
>       docElement.setAttribute("title",
>         docElement.getAttribute("title_privatebrowsing"));
>       docElement.setAttribute("titlemodifier",
>         docElement.getAttribute("titlemodifier_privatebrowsing"));
>-      docElement.setAttribute("browsingmode", "private");
>+      docElement.setAttribute("privatebrowsingmode", "temporary");
>       gBrowser.updateTitlebar();
>     }

Looks like docElement.setAttribute("privatebrowsingmode", "permanent") should depend on (window.location.href == getBrowserURL()) as well.

>     this._setPBMenuTitle("start");
> 
>+    let docElement = document.documentElement;
>     if (window.location.href == getBrowserURL()) {
>       // Adjust the window's title
>-      let docElement = document.documentElement;
>       docElement.setAttribute("title",
>         docElement.getAttribute("title_normal"));
>       docElement.setAttribute("titlemodifier",
>         docElement.getAttribute("titlemodifier_normal"));
>-      docElement.setAttribute("browsingmode", "normal");
>     }
> 
>     // Enable the menu item in after exiting the auto-start mode
>     document.getElementById("privateBrowsingItem")
>             .removeAttribute("disabled");
> #ifdef MENUBAR_CAN_AUTOHIDE
>     document.getElementById("appmenu_privateBrowsing")
>             .removeAttribute("disabled");
> #endif
>     document.getElementById("Tools:PrivateBrowsing")
>             .removeAttribute("disabled");
>+    docElement.removeAttribute("privatebrowsingmode");

Here you're removing the dependency on (window.location.href == getBrowserURL()), which looks wrong.
Comment 37 :Ehsan Akhgari 2010-09-21 15:53:42 PDT
Created attachment 477325 [details] [diff] [review]
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root
Comment 38 Dão Gottwald [:dao] 2010-09-22 06:25:44 PDT
Comment on attachment 477325 [details] [diff] [review]
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

>+    let docElement = document.documentElement;
>     if (this._privateBrowsingService.autoStarted) {
>       // Disable the menu item in auto-start mode
>       document.getElementById("privateBrowsingItem")
>               .setAttribute("disabled", "true");
> #ifdef MENUBAR_CAN_AUTOHIDE
>       document.getElementById("appmenu_privateBrowsing")
>               .setAttribute("disabled", "true");
> #endif
>       document.getElementById("Tools:PrivateBrowsing")
>               .setAttribute("disabled", "true");
>+      if (window.location.href == getBrowserURL()) {
>+        docElement.setAttribute("privatebrowsingmode", "permanent");
>+      }

nit: drop the braces

>+    let docElement = document.documentElement;
>     if (window.location.href == getBrowserURL()) {
>       // Adjust the window's title
>-      let docElement = document.documentElement;
>       docElement.setAttribute("title",
>         docElement.getAttribute("title_normal"));
>       docElement.setAttribute("titlemodifier",
>         docElement.getAttribute("titlemodifier_normal"));
>-      docElement.setAttribute("browsingmode", "normal");
>     }
> 
>     // Enable the menu item in after exiting the auto-start mode
>     document.getElementById("privateBrowsingItem")
>             .removeAttribute("disabled");
> #ifdef MENUBAR_CAN_AUTOHIDE
>     document.getElementById("appmenu_privateBrowsing")
>             .removeAttribute("disabled");
> #endif
>     document.getElementById("Tools:PrivateBrowsing")
>             .removeAttribute("disabled");
>+    if (window.location.href == getBrowserURL()) {
>+      docElement.removeAttribute("privatebrowsingmode");
>+    }

It looks like there's no reason to shuffle this, i.e. you should just replace docElement.setAttribute("browsingmode", "normal"); with docElement.removeAttribute("privatebrowsingmode");.
Comment 39 :Ehsan Akhgari 2010-09-22 10:10:54 PDT
Created attachment 477544 [details] [diff] [review]
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

With nits addressed.

Please note that the part 2 patch is still awaiting review.
Comment 40 Dão Gottwald [:dao] 2010-09-22 11:19:07 PDT
Comment on attachment 477544 [details] [diff] [review]
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

>+    let docElement = document.documentElement;
>     if (window.location.href == getBrowserURL()) {
>       // Adjust the window's title
>-      let docElement = document.documentElement;
>       docElement.setAttribute("title",
>         docElement.getAttribute("title_normal"));
>       docElement.setAttribute("titlemodifier",
>         docElement.getAttribute("titlemodifier_normal"));
>-      docElement.setAttribute("browsingmode", "normal");
>+      docElement.removeAttribute("privatebrowsingmode");
>     }

bogus change around let docElement
Comment 41 :Ehsan Akhgari 2010-09-22 14:24:40 PDT
Created attachment 477687 [details] [diff] [review]
Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

Ah, right.  Fixed.
Comment 42 :Ehsan Akhgari 2010-10-07 10:28:26 PDT
dao: ping?
Comment 43 Jim Mathies [:jimm] 2010-10-18 07:10:29 PDT
*** Bug 605026 has been marked as a duplicate of this bug. ***
Comment 44 Dão Gottwald [:dao] 2010-10-19 07:42:41 PDT
Comment on attachment 477229 [details] [diff] [review]
Part 2: actually make the button purple

>--- a/browser/themes/winstripe/browser/browser-aero.css
>+++ b/browser/themes/winstripe/browser/browser-aero.css

>+  #main-window[privatebrowsingmode=temporary] #appmenu-button {
>+    -moz-border-left-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
>+    -moz-border-bottom-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
>+    -moz-border-right-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
>+  }
>+
>   #appmenu-button:-moz-window-inactive {
>     -moz-border-left-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
>     -moz-border-bottom-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
>     -moz-border-right-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
>     box-shadow: 0 0 0 1px rgba(255,255,255,.25) inset;
>   }

Does #appmenu-button:-moz-window-inactive take precedence over #main-window[privatebrowsingmode=temporary] #appmenu-button?

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

>-#appmenu-button:-moz-window-inactive {
>+#main-window[privatebrowsingmode=temporary] #appmenu-button {
>+  background-image: -moz-linear-gradient(rgb(153,38,211), rgb(105,19,163) 95%);
>+  border-color: rgba(43,8,65,.9);
>+}

Can you add :not(:-moz-window-inactive) here? (Though if the answer to the above question is "yes", this may not be needed.)

>+#appmenu-button:-moz-window-inactive,
>+#main-window[privatebrowsingmode=temporary] #appmenu-button:-moz-window-inactive {
>   background-image: none;
>   border-color: rgba(0,0,0,.4);
> }

And drop #main-window[privatebrowsingmode=temporary] #appmenu-button:-moz-window-inactive here?
Comment 45 :Ehsan Akhgari 2010-10-20 09:21:57 PDT
(In reply to comment #44)
> Comment on attachment 477229 [details] [diff] [review]
>   --> https://bugzilla.mozilla.org/attachment.cgi?id=477229
> Part 2: actually make the button purple
> 
> >--- a/browser/themes/winstripe/browser/browser-aero.css
> >+++ b/browser/themes/winstripe/browser/browser-aero.css
> 
> >+  #main-window[privatebrowsingmode=temporary] #appmenu-button {
> >+    -moz-border-left-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> >+    -moz-border-bottom-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> >+    -moz-border-right-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> >+  }
> >+
> >   #appmenu-button:-moz-window-inactive {
> >     -moz-border-left-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> >     -moz-border-bottom-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> >     -moz-border-right-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> >     box-shadow: 0 0 0 1px rgba(255,255,255,.25) inset;
> >   }
> 
> Does #appmenu-button:-moz-window-inactive take precedence over
> #main-window[privatebrowsingmode=temporary] #appmenu-button?

The specificity of the former is (a=0, b=1, c=1, d=0), and the specificity of the latter is (a=0, b=2, c=1, d=0), therefore the latter takes precedence on the former.

> >--- a/browser/themes/winstripe/browser/browser.css
> >+++ b/browser/themes/winstripe/browser/browser.css
> 
> >-#appmenu-button:-moz-window-inactive {
> >+#main-window[privatebrowsingmode=temporary] #appmenu-button {
> >+  background-image: -moz-linear-gradient(rgb(153,38,211), rgb(105,19,163) 95%);
> >+  border-color: rgba(43,8,65,.9);
> >+}
> 
> Can you add :not(:-moz-window-inactive) here? (Though if the answer to the
> above question is "yes", this may not be needed.)

Well, the answer to the above question was no, but I don't see why the :not rule would be needed here.  This selector takes precedence over #appmenu-button:-moz-window-inactive, which is what we want, right?

> >+#appmenu-button:-moz-window-inactive,
> >+#main-window[privatebrowsingmode=temporary] #appmenu-button:-moz-window-inactive {
> >   background-image: none;
> >   border-color: rgba(0,0,0,.4);
> > }
> 
> And drop #main-window[privatebrowsingmode=temporary]
> #appmenu-button:-moz-window-inactive here?

This will cause the border-color rule from the previous selector to be applied to that case.  Is that what we really want?
Comment 46 Dão Gottwald [:dao] 2010-10-20 11:44:10 PDT
(In reply to comment #45)
> > And drop #main-window[privatebrowsingmode=temporary]
> > #appmenu-button:-moz-window-inactive here?
> 
> This will cause the border-color rule from the previous selector to be applied
> to that case.  Is that what we really want?

No, which is why I suggested adding :not(:-moz-window-inactive) there.
Comment 47 :Ehsan Akhgari 2010-10-20 16:15:10 PDT
Created attachment 484869 [details] [diff] [review]
Part 2: actually make the button purple

With comments addressed.
Comment 48 Dão Gottwald [:dao] 2010-10-21 08:42:30 PDT
(In reply to comment #45)
> (In reply to comment #44)
> > Comment on attachment 477229 [details] [diff] [review]
> >   --> https://bugzilla.mozilla.org/attachment.cgi?id=477229
> > Part 2: actually make the button purple
> > 
> > >--- a/browser/themes/winstripe/browser/browser-aero.css
> > >+++ b/browser/themes/winstripe/browser/browser-aero.css
> > 
> > >+  #main-window[privatebrowsingmode=temporary] #appmenu-button {
> > >+    -moz-border-left-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> > >+    -moz-border-bottom-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> > >+    -moz-border-right-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> > >+  }
> > >+
> > >   #appmenu-button:-moz-window-inactive {
> > >     -moz-border-left-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> > >     -moz-border-bottom-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> > >     -moz-border-right-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> > >     box-shadow: 0 0 0 1px rgba(255,255,255,.25) inset;
> > >   }
> > 
> > Does #appmenu-button:-moz-window-inactive take precedence over
> > #main-window[privatebrowsingmode=temporary] #appmenu-button?
> 
> The specificity of the former is (a=0, b=1, c=1, d=0), and the specificity of
> the latter is (a=0, b=2, c=1, d=0), therefore the latter takes precedence on
> the former.

Wait... does former/latter refer to the CSS source or to what I wrote?
Comment 49 :Ehsan Akhgari 2010-10-21 12:47:51 PDT
(In reply to comment #48)
> (In reply to comment #45)
> > (In reply to comment #44)
> > > Comment on attachment 477229 [details] [diff] [review]
> > >   --> https://bugzilla.mozilla.org/attachment.cgi?id=477229
> > > Part 2: actually make the button purple
> > > 
> > > >--- a/browser/themes/winstripe/browser/browser-aero.css
> > > >+++ b/browser/themes/winstripe/browser/browser-aero.css
> > > 
> > > >+  #main-window[privatebrowsingmode=temporary] #appmenu-button {
> > > >+    -moz-border-left-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> > > >+    -moz-border-bottom-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> > > >+    -moz-border-right-colors: rgba(255,255,255,.5) rgba(43,8,65,.9);
> > > >+  }
> > > >+
> > > >   #appmenu-button:-moz-window-inactive {
> > > >     -moz-border-left-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> > > >     -moz-border-bottom-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> > > >     -moz-border-right-colors: rgba(255,255,255,.4) rgba(0,0,0,.5);
> > > >     box-shadow: 0 0 0 1px rgba(255,255,255,.25) inset;
> > > >   }
> > > 
> > > Does #appmenu-button:-moz-window-inactive take precedence over
> > > #main-window[privatebrowsingmode=temporary] #appmenu-button?
> > 
> > The specificity of the former is (a=0, b=1, c=1, d=0), and the specificity of
> > the latter is (a=0, b=2, c=1, d=0), therefore the latter takes precedence on
> > the former.
> 
> Wait... does former/latter refer to the CSS source or to what I wrote?

What you wrote.  (I resisted the temptation of saying "the latter" again! ;-) )
Comment 50 Dão Gottwald [:dao] 2010-10-21 12:51:55 PDT
It seems to me that the :-moz-window-inactive rule should take precedence, i.e. you should add :not(:-moz-window-inactive) to the other selector just like in browser.css. By the way, please add that to #appmenu-button rather than #main-window.
Comment 51 :Ehsan Akhgari 2010-10-21 21:47:05 PDT
(In reply to comment #50)
> It seems to me that the :-moz-window-inactive rule should take precedence, i.e.
> you should add :not(:-moz-window-inactive) to the other selector just like in
> browser.css. By the way, please add that to #appmenu-button rather than
> #main-window.

I'm kind of lost what you mean here.  I used the specificity of the selectors as defined in the CSS spec.  And I'm not sure what we're trying to fix here... :/
Comment 52 Dão Gottwald [:dao] 2010-10-21 23:01:08 PDT
(In reply to comment #51)
> (In reply to comment #50)
> > It seems to me that the :-moz-window-inactive rule should take precedence, i.e.
> > you should add :not(:-moz-window-inactive) to the other selector just like in
> > browser.css. By the way, please add that to #appmenu-button rather than
> > #main-window.
> 
> I'm kind of lost what you mean here.  I used the specificity of the selectors
> as defined in the CSS spec.

You're saying the privatebrowsingmode=temporary styling overrides the window-inactive styling. (Aren't you?) I'm saying we probably want it the other way around.
Comment 53 :Ehsan Akhgari 2010-10-22 08:29:49 PDT
How about modifying #appmenu-button:-moz-window-inactive to #main-window #appmenu-button:-moz-window-inactive so that the two rules become similarly specific, and the second one would override the first one?
Comment 54 Dão Gottwald [:dao] 2010-10-22 08:32:16 PDT
You think that's better than adding :not(:-moz-window-inactive) to the first selector?
Comment 55 :Ehsan Akhgari 2010-10-22 11:57:00 PDT
(In reply to comment #54)
> You think that's better than adding :not(:-moz-window-inactive) to the first
> selector?

Adding :not(:-moz-window-inactive) to the first selector doesn't help anything, it only makes that selector more specific than it already is.  Or am I missing something?
Comment 56 Dão Gottwald [:dao] 2010-10-23 04:11:50 PDT
It makes the selector not match in the window-inactive case, specificity doesn't even matter then.
Comment 57 :Ehsan Akhgari 2010-10-25 09:06:00 PDT
Created attachment 485746 [details] [diff] [review]
Part 2: actually make the button purple

OK, this should address your comments.
Comment 59 Eric Shepherd [:sheppy] 2010-11-05 12:46:27 PDT
Documented the API change here:

https://developer.mozilla.org/En/Supporting_private_browsing_mode#Detecting_whether_private_browsing_mode_is_permanent

Added an example of skinning the firefox button using the privatebrowsingmode attribute here:

https://developer.mozilla.org/En/Supporting_private_browsing_mode#Private_browsing_for_theme_designers
Comment 60 Marcia Knous [:marcia - use ni] 2010-11-10 15:42:52 PST
https://litmus.mozilla.org/show_test.cgi?id=13805 to Litmus.
Comment 61 Dan Jacobson 2014-03-08 03:28:24 PST
All I know is I went to the public library where they use Windows, and hit CTRL-N which I recall had the same golden color as the non-private window. I recall there was a message inside the window saying that it was a private window. I double checked that I was using indeed the latest Firefox 27.

Isn't it a security issue, making private windows look like non-private windows?
Comment 62 Dan Jacobson 2014-03-08 03:31:05 PST
I think I mean to say that choosing private browsing from the menus makes the same colored windows as the usual CTRL-N non-private windows.
Comment 63 Dan Jacobson 2014-03-08 03:32:16 PST
P.S. and why have things look so different on Linux?
Comment 64 Stephen Horlander [:shorlander] 2014-03-08 07:59:11 PST
(In reply to Dan Jacobson from comment #61)
> All I know is I went to the public library where they use Windows, and hit
> CTRL-N which I recall had the same golden color as the non-private window. I
> recall there was a message inside the window saying that it was a private
> window. I double checked that I was using indeed the latest Firefox 27.

It is possible to set browser.privatebrowsing.autostart to true in about:config. This makes private browsing mode the default state. Could be what you were seeing.

> Isn't it a security issue, making private windows look like non-private
> windows?

Potentially a mode error, but I don't see how it is a security issue.
Comment 65 Dan Jacobson 2014-03-08 15:31:38 PST
Well OK but I recall they were all orange color, not purple.

OK next time I go to town I will try to get more details.

If a danger signal is always on even though there is no danger, one day
when there is danger, users will ignore it.

If there is no security issue in both modes being the same color, then
one might ask why bother to distinguish them in the first place...
Comment 66 Dan Jacobson 2014-04-29 23:00:30 PDT
Confirmed no problem seen in Windows FF 7.0,1 or 12.0.

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