473860 - Closing window after leaving Private browsing mode still shows "Stop Private Browsing" in Tools menu

Status: VERIFIED FIXED

(Firefox :: Private Browsing, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090115 Shiretoko/3.1b3pre Ubiquity/0.1.4 ID:20090115020658

With the following steps we have an inconsistent behavior on OS X. Even the user is not in PB mode the tools menu item shows "Stop Private Browsing".

Steps:
1. Enter Private Browsing mode
2. Leave Private Browsing mode
3. Open the tools menu => "Start Private Browsing" is visible
4. Close Window
3. Open the tools menu => "Stop Private Browsing" is visible

No idea if this is really related to bug 463582. It's your decision Ehsan.

Comment 1

[Aakash Desai [:aakashd]]

Test case https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7471 has been created for regression testing this bug.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I've disabled the test until this bug will be fixed.

Comment 3

[(no longer active)]

Attachment: Patch (v1)

Henrik, can you please try this patch and see if it fixes the problem?  If you don't want to build, just go to firefox-dir/chrome, extract browser.jar and apply the patch to browser.js manually and re-zip it.

Thanks!

Comment 4

[(no longer active)]

Attachment: Patch (v1)

Sorry the previous patch contained some unwanted changes.  This one is the right patch.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I've created a tryserver build with the latest patch which is available here:
https://build.mozilla.org/tryserver-builds/2009-01-22_01:[e-mail]-bug473860/

For me the "Stop Private Browsing" label is still shown. There is no change with this patch. Anyone else running OS X can check it too? Thanks.

Comment 6

[Mike Connor [:mconnor]]

That change is in init()... how would that work?  onExitPrivateBrowsing should just work, but maybe we throw at http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6942 in the non-browser window case?

Comment 7

[Mike Connor [:mconnor]]

Attachment: only reset gFindbar if it's not null

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 364219 [details] [diff] [review]
only reset gFindbar if it's not null

a191=beltzner

Comment 9

[(no longer active)]

Attachment: do the same for the import menu item as well

Bug 464736 added some similar code which should also be patched here.

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 364509 [details] [diff] [review]
do the same for the import menu item as well

the null checks on menuitems aren't needed, as the menuitems are should always be available.

Comment 11

[(no longer active)]

Comment on attachment 364509 [details] [diff] [review]
do the same for the import menu item as well

Ah, right.  I got too excited.  ;-)

Do you want me to land the previous patch for you?

Comment 12

[Mike Connor [:mconnor]]

Feel free.

Comment 13

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/9a80e846a26d

Comment 14

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a22fd422dacc

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified on trunk and 1.9.1 branch:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090228 Minefield/3.2a1pre ID:20090228023218

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090301 Shiretoko/3.1b3pre ID:20090301020400

Comment 16

[Marcia Knous [:marcia]]

I renabled the litmus test case now that is has been fixed and verified.

Comment 17

[jaykert]

jui