Private Browsing, which is currently being worked on in bug 248970, needs UI elements. The provided UI should at the very least enable the user to enter/exit the Private Browsing mode, and provide some kind of feedback to the user about the current state of the browser. faaborg is in the process of creating some mockups. The mockups will be posted here.

It could go on the Tools menu, right under the Clear Private Data item. It could just say "/x/ Private Browsing Enabled" where x is a checkmark. Of course that would not be visible all the time, but maybe that would be a good thing, to keep shoulder surfers from noticing.

Alex, could we get some mockups for this? Since the l10n freeze is pretty close, I thought maybe the mockup could determine which strings we would need, so that we can check those strings in and use them later in the UI code. Thanks!

>could we get some mockups for this? I need to prioritize getting the new windows theme landed. I'm really sorry I can't be of more help, this is a feature that I personally _really_ want us to include in Firefox 3, and something I've been advocating for a long time. I'll be sure to follow up in this bug once I have free cycles, and even if this doesn't make the cutoff for Firefox 3, I hope it is one of the first things we land on the trunk for Firefox 4.

(In reply to comment #3) > >could we get some mockups for this? > > I need to prioritize getting the new windows theme landed. I'm really sorry I > can't be of more help, this is a feature that I personally _really_ want us to > include in Firefox 3, and something I've been advocating for a long time. > > I'll be sure to follow up in this bug once I have free cycles, and even if this > doesn't make the cutoff for Firefox 3, I hope it is one of the first things we > land on the trunk for Firefox 4. No problem, I totally understand the pressure of satisfying different/opposing constraints in a software project. Sorry that I've kept on bugging you on this, I'm happy that you're keeping this on track. :-) I'm also very eager to land this for Firefox 3, but I understood that it's kind of late in the release schedule to land such a feature from the moment that I started to work on this. I thought though, what the heck? let's give it a shot! ;-)

My take is: When disabled ... * add a menuItem in the Tools menu: "Private Browsing..." entering the mode should contain a confirmation dialog (similar to Safari) which explains what features of the browser will be disabled (we can wordsmith this when bug 248970 is finished) and how the user can exit the mode. When enabled ... * change the menuItem in the Tools menu to be: "Stop Private Browsing" * in default theme, make the background of the location bar, the search bar and text entry fields in web pages a greyish white * add "Private Browsing Mode" to the status bar upon existing private browsing mode a confirmation dialog will remind the user what the change in their state will be. I'd previously thought about just having a button get added to the toolbar when the user was in Private Browsing mode, but eventually convinced myself that if they found the on switch, they should be able to find the off switch. Ambient status can be given by the slight darkening of the location bar (not so dark that people can see it from miles away) and other text entry fields will communicate that the user is in a different mode. (as always, I reserve the right to rethink this, but I'm pretty sure that this would work well!)

>* in default theme, make the background of the location bar, the search bar and >text entry fields in web pages a greyish white I'm concerned that changing the color of the location bar will falsely identify that the user has an anonymous connection to the site itself (like Tor), since the current yellow treatment (ssl) is about the user's connection to the site. I can see someone thinking that their employer can't track them because the location bar is ghosted (just as yellow means encrypted), and subsequently getting fired because their IP was tracked while they were whistle blowing or something. Since this is browser level anonymity, and not network anonymity, I think we should keep the cues to changes in chrome since this is conceptually more accurate. >* add "Private Browsing Mode" to the status bar This is fine, but I really want to get rid of the status bar so if we do have this, I wouldn't want it to be our only visual cue.

(In reply to comment #6) > >* in default theme, make the background of the location bar, the search bar and > >text entry fields in web pages a greyish white > > I'm concerned that changing the color of the location bar will falsely identify > that the user has an anonymous connection to the site itself (like Tor), since > the current yellow treatment (ssl) is about the user's connection to the site. Mmm, good point. I suppose we could add a glyph inside the location bar, though. > This is fine, but I really want to get rid of the status bar so if we do have > this, I wouldn't want it to be our only visual cue. One thing at a time, one thing at a time :)

Not likely to be ready in time for 3.1, probably 3.2 or so.

(In reply to comment #5) > I'd previously thought about just having a button get added to the toolbar when > the user was in Private Browsing mode, but eventually convinced myself that if > they found the on switch, they should be able to find the off switch. Just my .02: there should be an optional button (on/off switch) at least that can be placed on the toolbar for those in need of fast access. (this is prob obvious anyhow just thought I'd mention it).

there is a addon called "Stealther", finded at https://addons.mozilla.org/sv-SE/firefox/addon/1306 it have it's on/off switch in the tools menu, and you can choose how much you can hide...

Coming from someone who has little UX experience but doesn't act like it :) 1. We need to be careful about native widgets where we can't change properties like background or border. So changing the location bar would not only be a hassle but, 2. The notification should be subtle. A big change like the location bar colour would fail the over-the-shoulder test (although based on common uses of Private Browsing, the signal that they're in PB mode is the least of their worries if someone is looking over their shoulder, I suppose...). We must still meet the goal of a non-distracting chrome and one that is simple to implement and fast to render. My suggestion is to change the hue of the throbber. Its very easy to do in CSS, is subtle, and can still be noticed when you're really looking for it. Make the throbber dark blue instead of grey like it is now. Of course, ignore this post if you wish...

What is the current implementation, if any, for 3.1?

I would like to find out this as well in order to finish flushing out the test plan, and see if what mconnor states in Comment 8 is still the plan. (In reply to comment #12) > What is the current implementation, if any, for 3.1?

Alex and mconnor have been discussing this in email, and I've been a fly on the wall. My expectation is that design materials should be available quite soon.

>My expectation is that design materials should be available quite soon. Here is the current mockup of all privacy related UI, including private browsing mode: http://people.mozilla.com/~faaborg/files/shiretoko/privacy_i2.png

Here is the wording used by Safari, Chrome and IE8 just as a point of reference. The text in the most recent UI mockup (http://people.mozilla.com/~faaborg/files/shiretoko/privacy_i2.png) probably isn't final.

I've looked into this only briefly, but one thing caught my attention. The pref page UI seems to suggest that it's possible to put the browser into "always-private" mode, and selectively choose what kinds of data to remember. This is not possible with the current backend changes. I'm not saying that it's not possible at all; only we need to file some backend bugs and fix them if this mode is supposed to be implemented in this manner.

One other thing: I think we may need some kind of indicator in the primary UI in addition to the titlebar change, because I've seen many users who don't know that the title bar is part of the data that an application can represent. They even don't know where "page titles" (as shown in search engines for example) come from. Other users who know the purpose of the title bar are trained to look at the left side of the title text because they're used to the fact that the right part is always "Mozilla Firefox" (or "Minefield" for that matter). IOW, I think that with the current plan, a lot of users may feel lost as to whether they're in private browsing mode or not. Another thing is that the mockup does not specify what happens to the Tools menu entry after entering the private mode. Does it turn to "Stop Private Browsing..."? Also, when in the "always-private" mode, should the about:privatebrowsing page be displayed as a kind of pseudo-homepage, or should user's usual home page be shown instead?

>"always-private" mode, and selectively choose what kinds of data to remember. >This is not possible with the current backend changes. I'm not saying that >it's not possible at all; only we need to file some backend bugs and fix them >if this mode is supposed to be implemented in this manner. Ok, I'm not sure overall how important it actually is to let users be selective under that option, but a small set of users will likely appreciate the additional functionality. If we can't make the change in time we will just alter the UI to contain a message where the controls would otherwise be. >I think that with the current plan, a lot of users may feel lost as to >whether they're in private browsing mode or not. Yeah, I tend to agree, I'll give this some thought for the next iteration. It won't change any of the existing design work, just add to it. >Another thing is that the mockup does not specify what happens to the Tools >menu entry after entering the private mode. Does it turn to "Stop Private >Browsing..."? Yep, I forgot to add that. It toggles to "Stop Private Browsing" >Also, when in the "always-private" mode, should the about:privatebrowsing page >be displayed as a kind of pseudo-homepage, or should user's usual home page be >shown instead? Just the user's normal home page. Also, I don't think there shouldn't be any UI indicators, like the title change.

(In reply to comment #19) > Ok, I'm not sure overall how important it actually is to let users be selective > under that option, but a small set of users will likely appreciate the > additional functionality. If we can't make the change in time we will just > alter the UI to contain a message where the controls would otherwise be. OK, this might not be too difficult at this stage; I'll start filing some bugs for each module. I'm not sure if we can target this to Beta 2 (because review times may vary) but I'll do my best to prepare the patches pretty soon. Also, I'm not sure if it's something we can take after Beta 2... > Yeah, I tend to agree, I'll give this some thought for the next iteration. It > won't change any of the existing design work, just add to it. One possible idea might be to change the glow behind the favicon to something dark, but it may interact badly with the different colors of favicons available. Another idea that I was thinking of was to add an icon next to the start icon in the location bar, but I'm not sure if that's the best place, since the location bar is usually associated with the current page, while the private mode is global. Of course, the status bar may as well be our victim here! :-) > Yep, I forgot to add that. It toggles to "Stop Private Browsing" Without the ellipsis I assume (because there's no dialog displayed after it)? > Just the user's normal home page. Also, I don't think there shouldn't be any UI > indicators, like the title change. Oh, I didn't mean that we should remove UI indicators in that case, sorry for the confusion.

>Without the ellipsis I assume (because there's no dialog displayed after it)? right, without the ellipsis since there is no dialog >> Just the user's normal home page. Also, I don't think there shouldn't be any UI >> indicators, like the title change. wow, I need sleep :) meant to say "there should be no indicators." The user is going to always be in this mode, so indicators just add clutter, and at this level it is really more of a setting than a mode.

(In reply to comment #21) > wow, I need sleep :) meant to say "there should be no indicators." The user is > going to always be in this mode, so indicators just add clutter, and at this > level it is really more of a setting than a mode. What happen when the user puts the browser out of the private mode, and enters the private mode again later? Should the UI bits be shown in that case?

Another random thought: should private browsing mode clear the clipboard when exiting, if the contents were copied while in private mode? EG, user enters PB, copies a link on cheap-engagement-rings.com, then exits PB mode. That link remains in the clipboard, where someone might discover it if they paste it into something later on.

Bah, wrong tab and bug. Meant to add that comment to bug 248970.

OK, there's a lot of confusion in this bug from comment 17 onwards, about the preference in the privacy pane ("Firefox should not remember private data") which puts the browser into a permanent state of private browsing. * is that a mode, or are we just creating a single element of UI for turning off all data collection services? * how does the private browsing mode menuItem in the Tools menu react when this option is set? (From what I can tell, Alex is asserting in comment 21 that unlike the mode, when this preference is set there should be no additional indicators.) We should probably split the discussions into two bugs.

> * is that a mode, or are we just creating a single element of UI for turning >off all data collection services? Both, in addition to turning off all the things listed in the UI, it would also turn off some other things not listed in the UI (like cache). This is the computer in the hotel lobby mode, or computer lab mode. It won't ask to remember passwords, etc. But there aren't any UI indicators. > * how does the private browsing mode menuItem in the Tools menu react when >this option is set? I think I would keep it around since someone sitting down at a computer with the options set won't necessarily know that this instance of Firefox has been placed in a permanent private browsing mode. So under this approach you can still enter and exit private browsing mode in terms of the interface and UI indicators, but this doesn't actually achieve anything since the computer was always in private browsing mode to begin with. Another option is to check the private browsing mode menu item, and then disable it. The benefit here is that the browser does a better job of explaining its state to the user, but it might not be immediately clear how the browser go into this state, or how to undo it.

(In reply to comment #26) > Both, in addition to turning off all the things listed in the UI, it would also > turn off some other things not listed in the UI (like cache). This is the > computer in the hotel lobby mode, or computer lab mode. It won't ask to > remember passwords, etc. But there aren't any UI indicators. Let's say someone does the below steps. Please correct me where I'm wrong in showing the UI indicators. 1. Starts Firefox (w/o indicators). 2. Enters PB (w/ indicators). 3. Sets the Always-PB pref (w/ indicators). 4. Closes the private window (the non-private windows are restored) (w/o indicators) 5. Exits Firefox. 6. Starts Firefox (automatically put in private mode) (w/o indicators) 7. Leaves PB (w/o indicators) 8. Enters PB (w/ indicators) 9. Closes the private window (w/o indicators) 10. Exits Firefox. > I think I would keep it around since someone sitting down at a computer with > the options set won't necessarily know that this instance of Firefox has been > placed in a permanent private browsing mode. So under this approach you can > still enter and exit private browsing mode in terms of the interface and UI > indicators, but this doesn't actually achieve anything since the computer was > always in private browsing mode to begin with. Hmmm, you mean no UI indicators are shown when that pref is set, even *if* the user leaves PB and enters it again? > Another option is to check the private browsing mode menu item, and then > disable it. The benefit here is that the browser does a better job of > explaining its state to the user, but it might not be immediately clear how the > browser go into this state, or how to undo it. Yeah, I think disabling the menu item would not be the correct choice. How about altering its text to something like "Automatic Private Browsing Mode" (possibly something less verbose though)?

>1. Starts Firefox (w/o indicators). >2. Enters PB (w/ indicators). >3. Sets the Always-PB pref (w/ indicators). >4. Closes the private window (the non-private windows are restored) (w/o >indicators) >5. Exits Firefox. >6. Starts Firefox (automatically put in private mode) (w/o indicators) >7. Leaves PB (w/o indicators) >8. Enters PB (w/ indicators) >9. Closes the private window (w/o indicators) >10. Exits Firefox. That all makes sense except for 6 to 7. If you start and are automatically put in, then how is the user leaving in step 7? >I think disabling the menu item would not be the correct choice. Hm, after the confusion of the 10 step process above, I wonder if a checked and disabled menu item wouldn't be a cleaner way for the user to understand that their browser is in a permanent private browsing mode.

(In reply to comment #28) > That all makes sense except for 6 to 7. If you start and are automatically put > in, then how is the user leaving in step 7? By unchecking the menu item, if it's not disabled of course. > Hm, after the confusion of the 10 step process above, I wonder if a checked and > disabled menu item wouldn't be a cleaner way for the user to understand that > their browser is in a permanent private browsing mode. It's a better way to inform the user that they can't leave the PB mode. Usually disabled items don't stand for permanent settings I guess. And the big problem is that a disabled menu item doesn't tell the user that in fact there is a way to turn off the setting (via preferences dialog). All it communicates is that "you're inside the private browsing mode and you can't do anything about it", really.

Here's my work on the UI so far. This took a bit longer than I expected, and some things are not done yet, as I will mention below. But for the sake of the string freeze, I thought I'd put it up for an initial review. Things that are not yet working: * I couldn't figure out how to modify the window close behavior inside the private browsing mode to trigger leaving the private mode and restoring the session instead of closing the app. * The title modification is kind of flaky. Try for example entering the PB mode, and opening a new tab (the title suffix is not added). Now, try opening a new window (title is correct), and opening a new tab on that window. The suffix disappears, and if you switch to the first tab it's not shown as well. * The icons of course need to be changed! ;-) * Based on our email conversation, I've use "Private Browsing" as the menuitem text, with checkbox semantics. I've changed a small piece of text in about:privatebrowsing accordingly. * Autostart mode is not handled as specified (UI indicators appear; menuitem is not yet disabled). * The "Clear Recent History" button in about:privatebrowsing doesn't work yet (there's no dialog to open!) I've submitted a try server build with my full stack of PB patches which should appear in 1-2 hours. I'll get some sleep, so you need to watch the tinderbox to figure out when it's available! :-)

Oh, and of course some browser chrome tests for the UI. I had some in my old UI patch in bug 248970, but I didn't get the time to port it to the new UI code yet...

That'll be https://build.mozilla.org/tryserver-builds/2008-10-31_01:05-ehsan.akhgari@gmail.com-try-377d151d5da/ No Windows builds, yet, but Linux and Mac are up!

Yeah, I'm not sure why it's failing. I tried a couple of times and they both failed similarly: <http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1225414198.1225416016.2626.gz> <http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1225440358.1225441993.32021.gz> IINM, I'm not touching anything under content/. Do you have any ideas why the builds fail? (I've pushed a build with only my core patch to the try server and it seems to be going fine, so it should be something in later patches... :/)

Oh, I just noted that a previous build from jblandy on Windows has failed similarly <http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1225409278.1225411625.23497.gz>, so maybe it's a problem with the build machine? Who should I ping about this?

(In reply to comment #30) > Things that are not yet working: > > * I couldn't figure out how to modify the window close behavior inside the > private browsing mode to trigger leaving the private mode and restoring the > session instead of closing the app. I couldn't parse this. What do you need here? > * The title modification is kind of flaky. Try for example entering the PB > mode, and opening a new tab (the title suffix is not added). Now, try opening > a new window (title is correct), and opening a new tab on that window. The > suffix disappears, and if you switch to the first tab it's not shown as well. This is bug 454949. Need to chase down a review for that. > * Autostart mode is not handled as specified (UI indicators appear; menuitem is > not yet disabled). that's easy though, right? ;) > * The "Clear Recent History" button in about:privatebrowsing doesn't work yet > (there's no dialog to open!) disable it, and have johnath enable it when that lands?

(In reply to comment #35) > (In reply to comment #30) > > Things that are not yet working: > > > > * I couldn't figure out how to modify the window close behavior inside the > > private browsing mode to trigger leaving the private mode and restoring the > > session instead of closing the app. > > I couldn't parse this. What do you need here? I think he means he needs a way to make sure that clicking the X won't close the app, and instead exit private browsing mode as intended. I don't think that is possible within javascript and would require a lot of work to make it possible.

(In reply to comment #35) > (In reply to comment #30) > > Things that are not yet working: > > > > * I couldn't figure out how to modify the window close behavior inside the > > private browsing mode to trigger leaving the private mode and restoring the > > session instead of closing the app. > > I couldn't parse this. What do you need here? Michael Ventnor is right here. Please see Alex's mockup. He requires that closing the private browsing window should restore the user's session instead of closing the app. I of course think that he means closing the last window inside the private browsing mode... > > * The title modification is kind of flaky. Try for example entering the PB > > mode, and opening a new tab (the title suffix is not added). Now, try opening > > a new window (title is correct), and opening a new tab on that window. The > > suffix disappears, and if you switch to the first tab it's not shown as well. > > This is bug 454949. Need to chase down a review for that. It'd be great, thanks! > > * Autostart mode is not handled as specified (UI indicators appear; menuitem is > > not yet disabled). > > that's easy though, right? ;) Yeah, just didn't have enough time to look through it. > > * The "Clear Recent History" button in about:privatebrowsing doesn't work yet > > (there's no dialog to open!) > > disable it, and have johnath enable it when that lands? Sure.

Comment on attachment 345636 [details] [diff] [review] Patch (v1) there's some license headers which could use preprocessor to strip out the license headers, you can file a followup there >diff --git a/browser/base/content/browser-menubar.inc b/browser/base/content/browser-menubar.inc >--- a/browser/base/content/browser-menubar.inc >+++ b/browser/base/content/browser-menubar.inc >@@ -461,16 +461,22 @@ > #ifdef XP_WIN > <menuitem accesskey="&pageInfoCmd.accesskey;" label="&pageInfoCmd.label;" > command="View:PageInfo"/> > #else > <menuitem accesskey="&pageInfoCmd.accesskey;" label="&pageInfoCmd.label;" > key="key_viewInfo" command="View:PageInfo"/> > #endif > <menuseparator id="sanitizeSeparator"/> >+ <menuitem id="togglePBItem" privateBrowsingItem please, PB won't be obvious enough in CSS files, etc. >diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js >--- a/browser/base/content/browser.js >+++ b/browser/base/content/browser.js >+ >+ gPrivateBrowsingUI.uninit(); don't need the newline here. > init: function PBUI_init() { > else > this.onExitPrivateBrowsing(); We don't really need to call this, do we? _shouldEnter: function PBUI__shouldEnter() { Hmm, so does this mean we can have a dialog from here, and from downloads? That kinda sucks... > onEnterPrivateBrowsing: function PBUI_onEnterPrivateBrowsing() { >+ let pbMenuItem = document.getElementById("togglePBItem"); >+ if (pbMenuItem) >+ pbMenuItem.setAttribute("checked", "true"); >+ >+ // Adjust the window's title >+ let docElement = document.documentElement; >+ docElement.setAttribute("title", >+ docElement.getAttribute("title_privatebrowsing")); >+ docElement.setAttribute("titlemodifier", >+ docElement.getAttribute("titlemodifier_privatebrowsing")); > }, > > onExitPrivateBrowsing: function PBUI_onExitPrivateBrowsing() { >+ let pbMenuItem = document.getElementById("togglePBItem"); >+ if (pbMenuItem) >+ pbMenuItem.removeAttribute("checked"); >+ >+ // Adjust the window's title >+ let docElement = document.documentElement; >+ docElement.setAttribute("title", >+ docElement.getAttribute("title_normal")); >+ docElement.setAttribute("titlemodifier", >+ docElement.getAttribute("titlemodifier_normal")); > }, Okay.... this is going to yield wackiness on Mac. Basically, we do this fun maneuver where we copy titlemodifier to titledefault and then clear the value from titlemodifier. We do this so we only use that string when there's no title, otherwise we just use the document title, like a good little Mac app. As a result, you currently get some stuff like "Minefield - Minefield (Private Browsing)" where document.title is empty. Like on new tabs. We need to remove contenttitlesetting="true" in browser.xul (in progress as I write this!) for this to work reliably, but here goes for how to fix on Mac: Enter: set titlemodifier to titlemodifier_privatebrowsing set titledefault to "" Exit: set titlemodifier to "" set titledefault to titlemodifier_normal diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul >- titlemodifier="&mainWindow.title;" >+ title_normal="&mainWindow.title;" >+ title_privatebrowsing="&mainWindow.title; &mainWindowPrivateBrowsing.titlemodifier;" >+ titlemodifier="&mainWindow.titlemodifier;" >+ titlemodifier_normal="&mainWindow.titlemodifier;" >+ titlemodifier_privatebrowsing="&mainWindow.titlemodifier; &mainWindowPrivateBrowsing.titlemodifier;" Please, never do it this way. If you need to, do it in the .dtd, so that if localizers need to change the ordering for some reason, they can combine them the right way. >+++ b/browser/components/privatebrowsing/content/aboutPrivateBrowsing.js why not just have an inline <script> element here? A separate file is overkill. >diff --git a/browser/locales/en-US/chrome/browser/privatebrowsing.properties b/browser/locales/en-US/chrome/browser/privatebrowsing.properties >new file mode 100644 >--- /dev/null >+++ b/browser/locales/en-US/chrome/browser/privatebrowsing.properties >@@ -0,0 +1,6 @@ >+dialogTitle=Start Private Browsing >+ >+message=%S will save your current tabs for when you are done with your Private Browsing session. >+yesTitle=&Start Private Browsing >+noTitle=&Cancel >+neverAsk=&Do not show this message again I don't know if we need a separate .properties file here, we could just have it in browser.properties r=me with that stuff fixed

>closing the private browsing window should restore the user's session instead >of closing the app. I of course think that he means closing the last window >inside the private browsing mode... I should note that I'm actually not really sure this is the right thing to do, since it is hard to tell if the user's intention is to close down Firefox, or simply exit private browsing mode. One option could be to have different behaviors from the menu item and the window close control (menu item takes you back to Firefox, and window close control just shuts the application down). It is admittedly pretty weird for the window close control not to actually close the application, and I'm looking forward to getting feedback on this behavior in the beta. Hopefully if we can eventually get private browsing mode to work on a per window basis we won't have to make these kind of strange UI decisions. Examples include this, and also the save session dialog you get when you are entering private browsing mode.

>* The "Clear Recent History" button in about:privatebrowsing doesn't work yet >(there's no dialog to open!) I think there is now no chance of that making 3.1, but johnath will know more.

(In reply to comment #38) > (From update of attachment 345636 [details] [diff] [review]) > there's some license headers which could use preprocessor to strip out the > license headers, you can file a followup there Done: bug 462579. > >diff --git a/browser/base/content/browser-menubar.inc b/browser/base/content/browser-menubar.inc > >--- a/browser/base/content/browser-menubar.inc > >+++ b/browser/base/content/browser-menubar.inc > >@@ -461,16 +461,22 @@ > > #ifdef XP_WIN > > <menuitem accesskey="&pageInfoCmd.accesskey;" label="&pageInfoCmd.label;" > > command="View:PageInfo"/> > > #else > > <menuitem accesskey="&pageInfoCmd.accesskey;" label="&pageInfoCmd.label;" > > key="key_viewInfo" command="View:PageInfo"/> > > #endif > > <menuseparator id="sanitizeSeparator"/> > >+ <menuitem id="togglePBItem" > > privateBrowsingItem please, PB won't be obvious enough in CSS files, etc. Done. > >diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js > >--- a/browser/base/content/browser.js > >+++ b/browser/base/content/browser.js > > >+ > >+ gPrivateBrowsingUI.uninit(); > > don't need the newline here. Done. > > init: function PBUI_init() { > > > else > > this.onExitPrivateBrowsing(); > > We don't really need to call this, do we? You're right, we don't. > _shouldEnter: function PBUI__shouldEnter() { > > Hmm, so does this mean we can have a dialog from here, and from downloads? > That kinda sucks... This dialog is shown always before that dialog. If the user hits Start in this dialog, and has any non-resumable downloads, then she will be prompted with that dialog. What do you suggest to do here? > > onEnterPrivateBrowsing: function PBUI_onEnterPrivateBrowsing() { > >+ let pbMenuItem = document.getElementById("togglePBItem"); > >+ if (pbMenuItem) > >+ pbMenuItem.setAttribute("checked", "true"); > >+ > >+ // Adjust the window's title > >+ let docElement = document.documentElement; > >+ docElement.setAttribute("title", > >+ docElement.getAttribute("title_privatebrowsing")); > >+ docElement.setAttribute("titlemodifier", > >+ docElement.getAttribute("titlemodifier_privatebrowsing")); > > }, > > > > onExitPrivateBrowsing: function PBUI_onExitPrivateBrowsing() { > >+ let pbMenuItem = document.getElementById("togglePBItem"); > >+ if (pbMenuItem) > >+ pbMenuItem.removeAttribute("checked"); > >+ > >+ // Adjust the window's title > >+ let docElement = document.documentElement; > >+ docElement.setAttribute("title", > >+ docElement.getAttribute("title_normal")); > >+ docElement.setAttribute("titlemodifier", > >+ docElement.getAttribute("titlemodifier_normal")); > > }, > > Okay.... this is going to yield wackiness on Mac. Basically, we do this fun > maneuver > where we copy titlemodifier to titledefault and then clear the value from > titlemodifier. > We do this so we only use that string when there's no title, otherwise we just > use the > document title, like a good little Mac app. As a result, you currently get > some stuff like > "Minefield - Minefield (Private Browsing)" where document.title is empty. Like > on new tabs. > > We need to remove contenttitlesetting="true" in browser.xul (in progress as I > write this!) Yeah, but 454949 solved the flakiness of the title that I had previously observed. We should do the same thing for view-source windows, right? (see bug 462639) > for this to work reliably, but here goes for how to fix on Mac: > > Enter: > set titlemodifier to titlemodifier_privatebrowsing > set titledefault to "" > > Exit: > set titlemodifier to "" > set titledefault to titlemodifier_normal Done. Please test this on a Mac if possible, as I don't have access to one. > diff --git a/browser/base/content/browser.xul > b/browser/base/content/browser.xul > >- titlemodifier="&mainWindow.title;" > >+ title_normal="&mainWindow.title;" > >+ title_privatebrowsing="&mainWindow.title; &mainWindowPrivateBrowsing.titlemodifier;" > >+ titlemodifier="&mainWindow.titlemodifier;" > >+ titlemodifier_normal="&mainWindow.titlemodifier;" > >+ titlemodifier_privatebrowsing="&mainWindow.titlemodifier; &mainWindowPrivateBrowsing.titlemodifier;" > > Please, never do it this way. If you need to, do it in the .dtd, so that if > localizers need to change the ordering for some reason, they can combine them > the right way. Done. > >+++ b/browser/components/privatebrowsing/content/aboutPrivateBrowsing.js > > why not just have an inline <script> element here? A separate file is > overkill. I did that with the exception that johnath might be doing something non-trivial here. But now that the button should be disabled for now, and may even be removed in the final release, I got rid of the script altogether! > I don't know if we need a separate .properties file here, we could just have it > in browser.properties Done. I also added a couple of localization notes. > r=me with that stuff fixed Thanks! Please review the titlemodifier related changes for Mac on this patch. I'll submit further changes as another patch on top of this one to make your job in reviewing them easier.

(In reply to comment #39) > I should note that I'm actually not really sure this is the right thing to do, > since it is hard to tell if the user's intention is to close down Firefox, or > simply exit private browsing mode. And I guess adding yet another modal dialog to ask the user their purpose isn't the best remedy as well. > One option could be to have different behaviors from the menu item and the > window close control (menu item takes you back to Firefox, and window close > control just shuts the application down). It is admittedly pretty weird for > the window close control not to actually close the application, and I'm looking > forward to getting feedback on this behavior in the beta. How about we add a toolbar button to the customization palette for controlling the private browsing mode for those who want to use something easier than the menu item? In addition to the benefit, this can be considered another kind of indicator for the private mode status in the primary UI (by keeping the button "pressed" while inside the mode. > Hopefully if we can eventually get private browsing mode to work on a per > window basis we won't have to make these kind of strange UI decisions. > Examples include this, and also the save session dialog you get when you are > entering private browsing mode. Yeah, that'd be just like how Chrome does it, but it might be hard to implement in the backend unless we have some kind of process-per-tab/process-per-window implementation...

(In reply to comment #40) > >* The "Clear Recent History" button in about:privatebrowsing doesn't work yet > >(there's no dialog to open!) > > I think there is now no chance of that making 3.1, but johnath will know more. Johnath: your input here is appreciated. Should I hide the related button and text from about:privatebrowsing for now?

>How about we add a toolbar button to the customization palette for controlling >the private browsing mode for those who want to use something easier than the >menu item? Hitting a toolbar button to close the browser and start a different session seems even stranger than relying on the window close button. I think there is an expectation that toolbar buttons act on the current window contents, at least I can't think of any counter examples. >> I think there is now no chance of that making 3.1, but johnath will know more. The clear recent history window implementation is bug 453440, Johnath just posted a WIP patch, so I really have no idea if it will make it in or not. >Should I hide the related button and text from about:privatebrowsing for now? Since we are now after string freeze, I'm not really sure. mconnor?

This patch handles the items which were left out of v1.0: * It adds a browser chrome test to test the essentials of the UI. * It enabled hiding the primary UI changes (which is in fact only the title change) when the private mode has been auto-started, and also disables the menu item. I had to add another attribute to nsIPrivateBrowsingService to detect whether we're in auto-started mode. The implementation is simple, and I also handled it in the existing PB unit tests. I'll ask r/sr on the IDL from bz if you show a green light on this change.

Comment on attachment 345852 [details] [diff] [review] Patch (v1.1) works just fine on Mac, which is all Ehsan needed to confirm, the requested changes look fine too.

Comment on attachment 345879 [details] [diff] [review] Remaining parts Asking r/sr from bz for the netwerk/ IDL changes. This should be quite trivial to review. :-)

Comment on attachment 345879 [details] [diff] [review] Remaining parts netwerk change looks ok

>+<!ENTITY privatebrowsingpage.clearRecentHistoryDesc "You may want to start >by also clearing your recent history."> That sounds a little strange to me ("also" in addition to what?), just a thought: maybe you could do without the also?

(In reply to comment #49) > >+<!ENTITY privatebrowsingpage.clearRecentHistoryDesc "You may want to start >by also clearing your recent history."> > That sounds a little strange to me ("also" in addition to what?), just a > thought: maybe you could do without the also? That's for cases where you visit DontWantOthersToKnowIWasHere.com and suddenly realize you're not in private browsing mode, so you go private. The "also" part refers to entering the private browsing mode.

(In reply to comment #43) > (In reply to comment #40) > > >* The "Clear Recent History" button in about:privatebrowsing doesn't work yet > > >(there's no dialog to open!) > > > > I think there is now no chance of that making 3.1, but johnath will know more. > > Johnath: your input here is appreciated. > > Should I hide the related button and text from about:privatebrowsing for now? An independent "Clear Recent History" dialog will not happen for 3.1. I don't really think we should land dead UI - I think I would rather us either: a) Modify the page to point to the existing "Clear Private Data" dialog, which will either be the existing dialog or a modified version of it, depending on my success over the next day or so. b) Remove the UI, and file a followup to add it back at some point when we feel that we have a good dialog to point to. My preference is (a) - it answers the dontwantitlogged.com use case almost as well (albeit not quite as well as a time-sensitive dialog would) and is a natural linkage anyhow.

I think we should go for (b) for 3.1, encouraging users to obliterate all of their history kills the functionality of the awesome bar, and isn't a good substitution for the original idea of supporting undo.

(In reply to comment #52) > I think we should go for (b) for 3.1, encouraging users to obliterate all of > their history kills the functionality of the awesome bar, and isn't a good > substitution for the original idea of supporting undo. Does that persist as your opinion in a world where bug 453440 lands? I would prefer to offer the CPD-with-timespan option than none at all, in the case we're anticipating here, where someone realizes a little too late that they wanted to be private.

>Does that persist as your opinion in a world where bug 453440 lands? No, if bug 453440 lands then I think we should definitely include the button (the only real difference is the lack of the list view UI in the window for selecting a range).

Right, I agree. So given all that, Ehsan, I think you should either land with the button hidden, or break the button code out and add it to bug 453440 so that it can be activated if/when that patch lands.

Landed as http://hg.mozilla.org/mozilla-central/rev/b2b3816d152f. Sorry I didn't catch the comments about removing the button. Filed bug 462996 as a follow-up for this. I'll try my best to get a review on it so that we can land it before tonight's nightly.

Feedback from a user at: <http://ehsanakhgari.org/blog/2008-11-04/dont-leave-trace-private-browsing-firefox#comment-107> However, there is something in the UI that doesn't behave the way I would expect it. Private Browsing opens a new window, you can actually see the old window being closed and the new window open up. So I would expect that closing that new window will switch off Private Browsing again. Instead, the browser exits and my old session isn't even restored when I start it again. Maybe we should set browser.startup.page to 3 (or do something equivalent) when entering the private browsing mode, so that at least the user's session is restored if they close Firefox?

(In reply to comment #57) > Maybe we should set browser.startup.page to 3 (or do something equivalent) when > entering the private browsing mode, so that at least the user's session is > restored if they close Firefox? Filed bug 463188 for this.

Over in bug 463436 Ehsan notes that we are not exiting private browsing mode when the user closes all of the private browsing windows. I'm concerned that users will be "caught" using private browsing mode when they close everything thinking they are finished with the session. The next user who launches Firefox will find that it opens up into private browsing mode, disclosing that the last person to use the computer is hiding something.

(In reply to comment #59) > Over in bug 463436 Ehsan notes that we are not exiting private browsing mode > when the user closes all of the private browsing windows. I'm concerned that > users will be "caught" using private browsing mode when they close everything > thinking they are finished with the session. The next user who launches Firefox > will find that it opens up into private browsing mode, disclosing that the last > person to use the computer is hiding something. The private browsing settings is not persisted across sessions (unless the autostart pref it set, which is a whole different matter) so this should not be a problem under Windows and Linux at least, where closing all browser windows quits Firefox. On Mac OS X however, I hear that it's possible to close all browser windows and still have Firefox running. I'm not sure if this problem manifests itself there, because I'm not familiar with that OS and I don't have a Mac to test it. Alex, do you think this is still a problem under Windows/Linux? How about Mac?

Ehsan: Regarding Comment 60, on Mac the Firefox browser still runs even when I close all windows in PB mode. So if someone does, and then selects the Minefield icon or launches a new window from the File menu, the home page will launch, but they will still be able to see things such as the download history that has gone on in that session.

(In reply to comment #61) > Ehsan: Regarding Comment 60, on Mac the Firefox browser still runs even when I > close all windows in PB mode. So if someone does, and then selects the > Minefield icon or launches a new window from the File menu, the home page will > launch, but they will still be able to see things such as the download history > that has gone on in that session. I see. Bug 464792 filed for this.

From <http://ehsanakhgari.org/blog/2008-11-04/dont-leave-trace-private-browsing-firefox#comment-195>: I find the "Start Private Browsing" dialog confusing. especially, what does "Cancel" do ? After giving this some thought, I agree with this user. The "Cancel" choice doesn't tell a lot "what" it's going to cancel. Alex, do you have any better suggestions?

Ehsan, Has there been thought into defining a state - a clear defined name, for the browser outside of private browsing mode? There might be a need for it such as the comment about "Cancel", (i.e., "Return to Public Browsing Mode", or "Regular" browsing?) Just a thought.

(In reply to comment #64) > Has there been thought into defining a state - a clear defined name, for the > browser outside of private browsing mode? There might be a need for it such as > the comment about "Cancel", (i.e., "Return to Public Browsing Mode", or > "Regular" browsing?) I'm not sure if we've come up with a unified name for non-private browsing mode, but based on your suggestion, I think "Continue Regular Browsing" might be a better choice, because we're technically not switching the mode at all, hence not "returning". But of course this might make the dialog too verbose, but then again we used to have such dialogs (the session restore prompt), which again we got rid of... Oh well. :-)

I think all of the various platform interface guidelines try to establish that the default action button should be descriptive (like "Save file" or "Start private browsing") while the button to not perform the default action should just be Cancel (instead of "don't save file" or "don't start private browsing"). I believe the rationale is to give the user an easily identifiable target to say "no." A more descriptive version of "cancel" would require the user to both read and understand the dialog, and then make an informed choice between the two options. There are of course other clues, like the "no" button not having a blueish color (OS X, Vista), and being always on the right, or always on the left, but these aren't as strong as always being consistently named "cancel."

The message we show in that dialog reads: "Minefield will save your current tabs for when you are done with your Private Browsing session." Now, answering "Cancel" can mean either "don't start private browsing" or "don't save my current tabs"... So if someone actually reads and tries to understand the dialog message, they may get confused. Maybe rewording the message could help here. Something along the lines of: --- Do you want to start Private Browsing? Minefield will close your current tabs and start a new private browsing session, but your current tabs will be saved for when you are done with your Private Browsing session. --- But that message is too long... :(

Good point, the overall issue here is that this is a question dialog box but we never ask a direct question in the text. Also, the title "start private browsing" on windows is easy to miss, especially when placed on glass in vista. Here is some new text: Title: "Start Private Browsing" (Windows / Linux only) Message Text: "Would you like to start Private Browsing?" Informative Text: "Minefield will save your current tabs for when you are done with your Private Browsing session." Action Button: "Start Private Browsing" Attached are Windows and OS X HIGs for reference of where the stuff goes. On OS X we should be using a pop up dialog in cases where there is more than one window open instead of a sheet (or just always using a pop up dialog). Also, we should bold the question (although I don't think we do this correctly anywhere else). Fun fact: on page 473 of the Vista HIG there is actually a usage pattern called the "Ulterior motive confirmation" (!)

(In reply to comment #68) > Created an attachment (id=348936) [details] > Question dialog HIGs for reference > > Good point, the overall issue here is that this is a question dialog box but we > never ask a direct question in the text. Also, the title "start private > browsing" on windows is easy to miss, especially when placed on glass in vista. > Here is some new text: > > Title: "Start Private Browsing" (Windows / Linux only) > > Message Text: "Would you like to start Private Browsing?" > > Informative Text: "Minefield will save your current tabs for when you are done > with your Private Browsing session." > > Action Button: "Start Private Browsing" OK. This should be spun off to a separate bug, right? > Attached are Windows and OS X HIGs for reference of where the stuff goes. On > OS X we should be using a pop up dialog in cases where there is more than one > window open instead of a sheet (or just always using a pop up dialog). Also, > we should bold the question (although I don't think we do this correctly > anywhere else). Hmmm, I don't think the prompts service allows us to follow these styles exactly. We have three choices here: 1. Fix nsIPromptService to allow "Informative Text", which can't happen in 1.9.1 time frame I guess. 2. Simply append the informative text to the message text together with a couple of line breaks. 3. Create our own dialog here, and possibly duplicate some stuff from the prompts service there. Which one do you think we should take?

(In reply to comment #68) > Attached are Windows and OS X HIGs for reference of where the stuff goes. On > OS X we should be using a pop up dialog in cases where there is more than one > window open instead of a sheet (or just always using a pop up dialog). Also, > we should bold the question (although I don't think we do this correctly > anywhere else). Oh, and I meant to ask this. How does the current dialog behave? If it behaves differently to what you're describing, how can I fix it?

Let's file a seperate bug, and just append the text for now. We can work on creating native dialog boxes later as a follow up. >How does the current dialog behave? If it >behaves differently to what you're describing, how can I fix it? As a sheet on the current window, which I would imagine isn't possible to fix unless we make some changes to nsIpromptService.

I wonder, if there's a dialog to confirm entering private browsing, should the Private Browsing Tools menu item have an ellipsis?

(In reply to comment #72) > I wonder, if there's a dialog to confirm entering private browsing, should the > Private Browsing Tools menu item have an ellipsis? We discussed this elsewhere (I can't remember which bug) and the decision was not to add ellipsis to this menu item, because it may be redundant if the pref to turn off showing that prompt has been set, and also the fact that we don't do this elsewhere in the UI where the action could require a mere confirmation, such as File -> Work Offline.

Another example would be File > Close, which may produce the save session dialog, but doesn't contain the ellipsis. I think ideally we would know if the dialog is going to come up, and if so add the ellipsis, to be completely correct.

UI work landed in beta 2. Verified on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2) Gecko/20081201 Firefox/3.1b2.

Mass moving of all Firefox::General private browsing bugs to Firefox::Private Browsing.

Ehsan, you have asked to add one or more tests to Litmus. What areas are already covered by the automated tests? Are there untested ones?

(In reply to comment #77) > Ehsan, you have asked to add one or more tests to Litmus. What areas are > already covered by the automated tests? Are there untested ones? Pretty much every aspect is being tested automatically, except that we can't yet handle the main browser window being closed in automated tests, so the functionality regarding saving and closing down the session and restoring it afterward needs Litmus tests. Also, we need to test whether about:privatebrowsing is actually opened in the private browsing session

I am setting in-Litmus+ to this since https://litmus.mozilla.org/show_test.cgi?id=7394 and several other test cases cover the functionality Ehsan notes in Comment 78.

I was just wondering: is there any specific reason not to use the currently free P (which was in use for Clear Private Data at the time of introducing Start/Stop Private Browsing) instead of B?

(In reply to comment #80) > I was just wondering: is there any specific reason not to use the currently > free P (which was in use for Clear Private Data at the time of introducing > Start/Stop Private Browsing) instead of B? Oh, that's because Firefox developers hate extension developers and the currently most popular extension happens to use the access key "B" :) But seriously, I don't think reusing access keys like this is nice. People who actually use access keys will automatically press "P" after upgrading to Firefox 3.1 to clear private data - and they will find themselves entering Private Browsing, without any questions they could decline. That's at least one reason I see not to use "P". But most likely the access key was simply chosen before the access key for "Clear Recent History" changed.

(In reply to comment #81) > (In reply to comment #80) > and they will find themselves entering > Private Browsing, without any questions they could decline. There actually is a prompt though.

(In reply to comment #81) > That's at least one reason I see not to use "P". But most likely the access key > was simply chosen before the access key for "Clear Recent History" changed. Yeah, that was the reason IIRC. (In reply to comment #82) > There actually is a prompt though. Which can be configured off. Anyway, this prompt features a Whatever button, so maybe it wouldn't be that much effective anyway.

>But seriously, I don't think reusing access keys like this is nice. People who >actually use access keys will automatically press "P" after upgrading to >Firefox 3.1 to clear private data - and they will find themselves entering >Private Browsing I'm a bit confused, I thought the command to enter Clear Private Data was always accelerator-shift-delete, and the new command for private browsing is accelerator-shift-P. Was accelerator-shift-P also directing the user to the Clear Private Data in Firefox 3? I'm not seeing that on OS X, and it isn't documented on support.mozilla.org

Alex, it had me confused as well at first - but Ton was asking about menu access keys, not hotkeys.

>Ton was asking about menu access keys, not hotkeys. Ok, that makes considerably more sense. I think we should give Private Browsing the P access key. In addition to being a more direct menu access key, anyone who was previously making regular use of the clear private data window will likely want to leverage Private Browsing, under the theory that it isn't really about when the data was removed (now preemptively instead of retroactively), but that the user wants to achieve privacy. The first time they invoke private browsing using the muscle memory of trying to get to a different part of the interface they might be slightly confused or frustrated with the change, but overall they should be able to adapt to a better model of dealing with privacy (going off the record, instead of shooting the reporter).

(In reply to comment #86) > Ok, that makes considerably more sense. I think we should give Private > Browsing the P access key. In addition to being a more direct menu access key, > anyone who was previously making regular use of the clear private data window > will likely want to leverage Private Browsing, under the theory that it isn't > really about when the data was removed (now preemptively instead of > retroactively), but that the user wants to achieve privacy. Given that we are in string freeze, do we still want this for 3.1? I personally think that we should take it in 3.1 if we want to take it at all.

Beltzner or Axel: can we correct an access key post string freeze before RC1?

http://mxr.mozilla.org/l10n-mozilla1.9.1/search?string=privateBrowsingCmd.stop&find=browser/browser.dtd shows the l10n status, some use localized accesskeys, some don't. I'm not sure which is carried over dominantly, using browsing or private as the word to access. And of course stop energy galore.

Can we get a separate bug for the accesskey change so that we can triage it for string freeze break next week?

(In reply to comment #90) > Can we get a separate bug for the accesskey change so that we can triage it for > string freeze break next week? Bug 473722 filed.