859326 - New Private Window entry in right click Dock Icon menu doesn't make sense in permanent private browsing

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[Tiziana Sellitto [:tiziana]]

Steps to reproduce:
1. Single Right Click the Firefox dock Icon on Permanent Private Browsing mode.

Actual results:
We see "New Window" and "New Private Window" menu and they both open a private window.

Expected results:
The "New Window" menu is not necessary.

I have tested that on 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20130407 Firefox/22.0
Build ID: 20130407004016 

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20100101 Firefox/21.0
Build ID: 20130401192816

and the current Firefox 20 on release channel.

Comment 1

[Josh Matthews [:jdm]]

This is a simple matter of adding another entry to the list in the patch in bug 845592. The mac dock menu is defined at http://mxr.mozilla.org/mozilla-central/source/browser/base/content/macBrowserOverlay.xul?force=1#55.

Comment 2

[(no longer active)]

(In reply to Josh Matthews [:jdm] from comment #1)
> This is a simple matter of adding another entry to the list in the patch in
> bug 845592. The mac dock menu is defined at
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/
> macBrowserOverlay.xul?force=1#55.

Hmm, no, since we need a special case for non-browser windows...  Patch coming up.

Comment 3

[(no longer active)]

Attachment: Patch (v1)

Comment 4

[Josh Matthews [:jdm]]

Comment on attachment 734651 [details] [diff] [review]
Patch (v1)

Review of attachment 734651 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/browser.js
@@ +1807,5 @@
>  
>      SocialUI.nonBrowserWindowInit();
>  
> +    if (PrivateBrowsingUtils.permanentPrivateBrowsing) {
> +      document.getElementById("macDockMenuNewWindow").hidden = true;

Will this break on non-mac platforms, or does the menu always exist somewhere?

Comment 5

[(no longer active)]

(In reply to Josh Matthews [:jdm] from comment #4)
> Comment on attachment 734651 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 734651 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/browser.js
> @@ +1807,5 @@
> >  
> >      SocialUI.nonBrowserWindowInit();
> >  
> > +    if (PrivateBrowsingUtils.permanentPrivateBrowsing) {
> > +      document.getElementById("macDockMenuNewWindow").hidden = true;
> 
> Will this break on non-mac platforms, or does the menu always exist
> somewhere?

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#1756

Comment 6

[Josh Matthews [:jdm]]

I don't know what to make of that link. Looking elsewhere, it appears that the overlay usually only exists on OS X.

Comment 7

[(no longer active)]

(In reply to Josh Matthews [:jdm] from comment #6)
> I don't know what to make of that link. Looking elsewhere, it appears that
> the overlay usually only exists on OS X.

The entire block of code that I patched in browser.js is Mac only, so this won't break on non-Mac platforms (the patch should be a full no-op there.)

Comment 8

[Josh Matthews [:jdm]]

Comment on attachment 734651 [details] [diff] [review]
Patch (v1)

Review of attachment 734651 [details] [diff] [review]:
-----------------------------------------------------------------

Ah. Apologies for being dense.

Comment 9

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/faf3b775851c

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/faf3b775851c