588655 - When in private browsing mode, make the Firefox button purple

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

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)]

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]]

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]]

Ehsan: can you take this?

Comment 4

[(no longer active)]

(In reply to comment #3)
> Ehsan: can you take this?

Sure.

Comment 5

[(no longer active)]

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]]

Attachment: Purple Firefox Button in PBM - 01

Make the Firefox Button purple when in Private Browsing mode.

Comment 7

[Stephen Horlander [:shorlander]]

Attachment: Screenshot of Purple Firefox Button

Comment 8

[Dão Gottwald [:dao]]

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]]

(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

[(no longer active)]

(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)]

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]]

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]]

(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]]

Nice idea. It would look unique and it is creative.

Comment 15

[Peter Henkel [:Terepin]]

(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]]

Attachment: Purple Firefox Button in PBM - 02

Updated to use #main-window[browsingmode=private]

Comment 17

[(no longer active)]

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]]

Is there a way to do that with CSS?

Comment 19

[(no longer active)]

Attachment: 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

[(no longer active)]

Attachment: Part 2: actually make the button purple

Comment 21

[(no longer active)]

Attachment: Part 2: actually make the button purple

With a small fix...

Comment 22

[(no longer active)]

Attachment: 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]]

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]]

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

[(no longer active)]

(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

[(no longer active)]

Attachment: Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

Comment 27

[(no longer active)]

Attachment: Part 2: actually make the button purple

Based on the new privatebrowsingmode attribute.

Comment 29

[Luke Iliffe (Harlequin99)]

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]]

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]]

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

[(no longer active)]

(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

[(no longer active)]

Attachment: Part 2: actually make the button purple

Comment 34

[Dão Gottwald [:dao]]

(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

[(no longer active)]

(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]]

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

[(no longer active)]

Attachment: Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

Comment 38

[Dão Gottwald [:dao]]

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

[(no longer active)]

Attachment: 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]]

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

[(no longer active)]

Attachment: Part 1: add a privatebrowsingmode=temporary/permanent attribute to browser.xul's doc root

Ah, right.  Fixed.

Comment 42

[(no longer active)]

dao: ping?

Comment 44

[Dão Gottwald [:dao]]

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

[(no longer active)]

(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]]

(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

[(no longer active)]

Attachment: Part 2: actually make the button purple

With comments addressed.

Comment 48

[Dão Gottwald [:dao]]

(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

[(no longer active)]

(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]]

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

[(no longer active)]

(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]]

(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

[(no longer active)]

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]]

You think that's better than adding :not(:-moz-window-inactive) to the first selector?

Comment 55

[(no longer active)]

(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]]

It makes the selector not match in the window-inactive case, specificity doesn't even matter then.

Comment 57

[(no longer active)]

Attachment: Part 2: actually make the button purple

OK, this should address your comments.

Comment 58

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/a0baa33cd4ca
http://hg.mozilla.org/mozilla-central/rev/ae29792742d8

Comment 59

[Eric Shepherd [:sheppy]]

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]]

https://litmus.mozilla.org/show_test.cgi?id=13805 to Litmus.

Comment 61

[Dan Jacobson]

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]

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]

P.S. and why have things look so different on Linux?

Comment 64

[Stephen Horlander [:shorlander]]

(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]

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]

Confirmed no problem seen in Windows FF 7.0,1 or 12.0.