Closed Bug 461625 Opened 11 years ago Closed 11 years ago

Hide the UI for saving permission manager entries in Private Browsing mode

Categories

(Firefox :: Private Browsing, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 3.1b2

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

(Keywords: privacy)

Attachments

(2 files, 2 obsolete files)

Based on bug 460342 comment 3: we need to hide the pieces of UI which allows saving permission manager entries while in the Private Browsing mode.  Here are the places that I can think of right now which we need to handle:

* "Block images from xyz.com" context menu entry
* "Block images from xyz.com" in the Page Info dialog, Media tab
* "always for this website" option for the popup blocking UI
* "Use my choice for all cookies from this site" option for the cookies lifetime confirmation dialog
* The permission control inside the Page Info dialog, Permissions tab

I'm not sure if we want to handle the last one, though.
(In reply to comment #0)
> * "Block images from xyz.com" context menu entry

Yes

> * "Block images from xyz.com" in the Page Info dialog, Media tab

No, this is something you're explicitly choosing, not in the context of normal browsing, so I think we should leave it unchanged.

> * "always for this website" option for the popup blocking UI

Yes.

> * "Use my choice for all cookies from this site" option for the cookies
> lifetime confirmation dialog

Yes.  (Semi-related, there was a desire to have a "remember for session" option for perms.

> * The permission control inside the Page Info dialog, Permissions tab

As I said before, we should leave this alone.
I'll take this.

(In reply to comment #1)
> > * "Use my choice for all cookies from this site" option for the cookies
> > lifetime confirmation dialog
> 
> Yes.  (Semi-related, there was a desire to have a "remember for session" option
> for perms.

Shouldn't be too hard to implement because of how the permissions manager component is architectured.  Is there a bug filed for that?  Once we have this, we might want to change the behavior here to only save for the current session instead of hiding the UI pieces.
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attached patch Patch (v1) (obsolete) — Splinter Review
This patch handles the three places mentioned in comment 1.  For those parts in the scope of browser.xul, I used the gPrivateBrowsingUI object to minimize the number of places where the PB service is actually queried.
Attachment #344825 - Flags: review?(mconnor)
Comment on attachment 344825 [details] [diff] [review]
Patch (v1)

>+  get currentStatus PBUI_get_currentStatus() {
>+    return this._privateBrowsingService.privateBrowsingEnabled;
>   }

currentStatus isn't really descriptive.  Its a boolean, so you want to either just make it privateBrowsingEnabled or something else along those lines.  currentStatus == true is wonderfully confusing.
Attachment #344825 - Flags: review?(mconnor) → review+
Attached patch For checkin (obsolete) — Splinter Review
OK, used privateBrowsingEnabled instead.
Attachment #344825 - Attachment is obsolete: true
Attachment #345449 - Flags: review+
Whiteboard: [pb-ready-for-landing]
The new patch disables the checkbox instead of hiding it, as per IRC discussions.
Attachment #345449 - Attachment is obsolete: true
Attachment #346111 - Flags: review?(mconnor)
Attachment #346111 - Flags: review?(mconnor) → review+
http://hg.mozilla.org/mozilla-central/rev/0d3b97cd7867

Marcia: needs a Litmus test.  I can probably get around to write an automated test for it as well if you feel it's worth it.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-litmus?
Resolution: --- → FIXED
Whiteboard: [pb-ready-for-landing]
Target Milestone: --- → Firefox 3.1b2
Feedback from a user at <http://ehsanakhgari.org/blog/2008-11-04/dont-leave-trace-private-browsing-firefox#comment-101>:


I have Tools > Options > Privacy > Cookies set to "Ask every time", and when I first visit a site and get the "Confirm setting cookie" dialog I either allow session cookies or deny cookies (and, rarely, allow permanent cookies) and remember the decision for each site. It's a hassle but lets me blacklist some sites and deny permanent cookies to most.

When I switch to private browsing, sites still ask me if I can set the cookie, but I can't check "Use my choice for all cookies from this site" because the checkbox is disabled So I keep getting the "Confirm setting cookie" dialog over and over.

I assume the handling of cookies in Private Browsing is "Allow for session". I would document that this is the setting in Private Browsing, that it can't be changed, and that "Ask every time" can't be set, and then never show the dialog in Private Browsing mode, regardless of the user's privacy settings. OR, you could try to create some hella complex options for private browsing...


I think his/her suggestion of never showing the dialog might be worth considering, because effectively we'd treat all cookies as session-only in the private browsing mode (as they're not remembers in future sessions).  What do you think, mconnor?
(In reply to comment #8)
> I think his/her suggestion of never showing the dialog might be worth
> considering, because effectively we'd treat all cookies as session-only in the
> private browsing mode (as they're not remembers in future sessions).  What do
> you think, mconnor?

Oh, not showing the dialog won't be an option because we still want to give users the ability to reject cookies.  Are there other options here?
https://litmus.mozilla.org/show_test.cgi?id=7397 added to the Litmus FFT. 

Also verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081105 Minefield/3.1b2pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081105 Minefield/3.1b2pre.  Cookies, popups and images are handled properly here as the menu items are disabled per Comment 1.
Status: RESOLVED → VERIFIED
Flags: in-litmus? → in-litmus+
Mass moving of all Firefox::General private browsing bugs to Firefox::Private Browsing.
Component: General → Private Browsing
QA Contact: general → private.browsing
Blocks: 479720
Flags: in-testsuite?
Ehsan, so what about the preferences dialog? While staying in Private Browsing mode I can allow sites to open pop-ups and all the other stuff. Do you only want to block actions which can be done in a second level by the user?
Attached patch Tests (v1)Splinter Review
I wrote a full test suite for all of the changes landed as part of this bug, so that regressions such as bug 518327 won't happen again.  The fix related to that bug plus its test is posted on that bug.
Attachment #404350 - Flags: review?(mconnor)
(In reply to comment #12)
> Ehsan, so what about the preferences dialog? While staying in Private Browsing
> mode I can allow sites to open pop-ups and all the other stuff. Do you only
> want to block actions which can be done in a second level by the user?

We decided to leave some options in the preferences window as well as the page info dialog available to the users, based on the premise that those areas are only touched by advanced users, who know that configuring those items inside the private browsing mode leaves permanent records of their action.

Does this answer your question?
Yes, it does. Thanks.
Attachment #404350 - Flags: review?(mconnor) → review+
Tests landed on trunk as:

http://hg.mozilla.org/mozilla-central/rev/eb9dc9dc24fa
Flags: in-testsuite? → in-testsuite+
Test landed on 1.9.1 as well:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0ae4f626127c
No longer blocks: 479720
Depends on: 560653
Depends on: 777875
Depends on: 778424
No longer depends on: 778424
No longer depends on: 777875
Now that we have the new "Per-Window Private Browsing", what's the expected behavior for:
1. Clear cache through Tools > Clear Recent History
2. Go to the File main menu and select "New Private Window" (or press Ctrl+Shift+P / Cmd+Shift+P).
3. Open the pop-ups test page: http://mozqa.com/data/firefox/popups/popup_trigger.html?count=3
4. Click on the top right notification and select "Show ..." for each pop up.
5. Close the private window

Actual results:
All the pop-ups remain opened after closing the private browsing window
That is the desired behavior.
You need to log in before you can comment on or make changes to this bug.