The default bug view has changed. See this FAQ.

Add File - New - Private Window menuitem

RESOLVED FIXED in seamonkey2.18

Status

SeaMonkey
UI Design
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

(Blocks: 1 bug)

unspecified
seamonkey2.18
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
This belongs below File - New - Browser Window. In order to achieve this for the navigator window itself, I have moved File - New - Browser Tab before them both.
(Assignee)

Comment 1

4 years ago
Created attachment 709499 [details] [diff] [review]
Proposed patch
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #709499 - Flags: review?(philip.chee)
Attachment #709499 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 2

4 years ago
Comment on attachment 709499 [details] [diff] [review]
Proposed patch

>+function OpenPrivateWindow()
>+{
>+  window.openDialog(getBrowserURL(), "_blank",
>+                    "private,chrome,all,dialog=no", "about:privatebrowsing");
>+}

I might want to tweak this for the "Open Link in Private Window" context menuitem, something like this:

function OpenPrivateWindow(aUrl, aDoc)
{
  var referrer = null;
  if (aDoc) {
    urlSecurityCheck(aURL, aDoc.nodePrincipal,
                     Components.interfaces.nsIScriptSecurityManager.STANDARD);
    referrer = aDoc.documentURIObject;
  } else if (!aUrl) {
    aUrl = "about:privatebrowsing";
  }
  window.openDialog(getBrowserURL(), "_blank",
                    "private,chrome,all,dialog=no", aUrl, null, referrer);
}
(Assignee)

Comment 3

4 years ago
Created attachment 713193 [details] [diff] [review]
Revised patch

Added non-private in more places to ensure that windows aren't opened privately by mistake. Also created openNewPrivateWith to share code with openNewWindowWith instead of copying it into OpenPrivateWindow (for use by the context menu in a followup bug).
Attachment #709499 - Attachment is obsolete: true
Attachment #709499 - Flags: review?(philip.chee)
Attachment #709499 - Flags: review?(iann_bugzilla)
Attachment #713193 - Flags: review?(philip.chee)
Attachment #713193 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

4 years ago
Blocks: 841230

Comment 4

4 years ago
Comment on attachment 713193 [details] [diff] [review]
Revised patch

> +          <menuitem id="menu_newPrivate"/>
....
> +    <key id="key_newPrivate"/>
....
> +  <command id="cmd_newPrivate"
> +           oncommand="OpenPrivateWindow();"/>
....

I prefer "menu_newPrivateWindow", "key_newPrivateWindow", "cmd_newPrivateWindow", etc

> +function OpenPrivateWindow()
> +{
> +  window.openDialog(getBrowserURL(), "_blank",
> +                    "private,chrome,all,dialog=no", "about:privatebrowsing");
> +}
> +
>  function OpenBrowserWindow()
[Consider supporting the Firefox syntax OpenBrowserWindow({private: true}) for extension compatibility]

> +    var features = "private,chrome,all,dialog=no";
> +    if (aType != kNewPrivate)
> +      features = "non-" + features;
This is too clever by half but it makes the code less readable. I prefer something clearer e.g.

var features = aType != kNewPrivate ? "non-private" : "private;
features += ",chrome,all,dialog=no"

> +<!ENTITY newPrivateCmd.accesskey "P">
>  <!ENTITY printSetupCmd.label "Page Setup">
>  <!ENTITY printSetupCmd.accesskey "u">
>  <!ENTITY printPreviewCmd.label "Print Preview">
>  <!ENTITY printPreviewCmd.accesskey "v">
>  <!ENTITY printCmd.label "Print">
Your patch seems to have swallowed the ellipsis in "Page Setup…" and "Print Preview…".

From Comment 2:
> function OpenPrivateWindow(aUrl, aDoc)
> {
>   var referrer = null;
>   if (aDoc) {
>     urlSecurityCheck(aURL, aDoc.nodePrincipal,
>                      Components.interfaces.nsIScriptSecurityManager.STANDARD);

AFAIK nsIScriptSecurityManager.STANDARD is the default.
Attachment #713193 - Flags: review?(philip.chee) → review+

Comment 5

4 years ago
Comment on attachment 713193 [details] [diff] [review]
Revised patch

> const kPrivateWindow = 5;

Fri Feb 15 2013 01:15:39
Error: ReferenceError: kNewPrivate is not defined
Source file: chrome://communicator/content/utilityOverlay.js
Line: 1050
Attachment #713193 - Flags: review+

Comment 6

4 years ago
> const kPrivateWindow = 5;
How about kNewPrivateWindow ?

Comment 7

4 years ago
> +<!ENTITY newPrivateCmd.label "Private Window">
> +<!ENTITY newPrivateCmd.key "P">
> +<!ENTITY newPrivateCmd.accesskey "P">
This access key clashes with File->New->Composer _P_age.
(Assignee)

Comment 8

4 years ago
(In reply to Philip Chee from comment #4)
> I prefer "menu_newPrivateWindow", "key_newPrivateWindow",
> "cmd_newPrivateWindow", etc
None of the related commands include the suffix "Window" ... perhaps we should rename the feature to "Stealth" or "Secret" or something?

> >  function OpenBrowserWindow()
> [Consider supporting the Firefox syntax OpenBrowserWindow({private: true})
> for extension compatibility]
Slippery slope...

> Your patch seems to have swallowed the ellipsis in "Page Setup…" and "Print
> Preview…".
It's because I copied it across VNC, which doesn't like ellipses. (I don't log in to Bugzilla on the PC on which I wrote the patch.)

> AFAIK nsIScriptSecurityManager.STANDARD is the default.
Well that call isn't there any more to avoid code duplication anyway.

(In reply to Philip Chee from comment #6)
> > const kPrivateWindow = 5;
Eek, I forgot to rename this with the others (late untested change, oops)

(In reply to Philip Chee from comment #7)
> > +<!ENTITY newPrivateCmd.accesskey "P">
> This access key clashes with File->New->Composer _P_age.
Oops. How about "W"?
(Assignee)

Comment 9

4 years ago
(In reply to Philip Chee from comment #4)
> (From update of attachment 713193 [details] [diff] [review])
> >  function OpenBrowserWindow()
> [Consider supporting the Firefox syntax OpenBrowserWindow({private: true})
> for extension compatibility]
Actually there's only one caller of OpenBrowserWindow, I might as well call openNewPrivateWith directly.
(Assignee)

Updated

4 years ago
Blocks: 841616
(Assignee)

Comment 10

4 years ago
(In reply to comment #9)
> (In reply to Philip Chee from comment #4)
> > (From update of attachment 713193 [details] [diff] [review])
> > >  function OpenBrowserWindow()
> > [Consider supporting the Firefox syntax OpenBrowserWindow({private: true})
> > for extension compatibility]
> Actually there's only one caller of OpenBrowserWindow, I might as well call
> openNewPrivateWith directly.
I mean only one caller of OpenPrivateWindow of course.

Comment 11

4 years ago
>>> +<!ENTITY newPrivateCmd.accesskey "P">
>> This access key clashes with File->New->Composer _P_age.
> Oops. How about "W"?
Sure.

>> Actually there's only one caller of OpenBrowserWindow, I might as well call
>> openNewPrivateWith directly.
> I mean only one caller of OpenPrivateWindow of course.
Sounds good.
(Assignee)

Comment 12

4 years ago
Created attachment 714347 [details] [diff] [review]
Addressed review comments
Attachment #714347 - Flags: review?(philip.chee)

Comment 13

4 years ago
Comment on attachment 714347 [details] [diff] [review]
Addressed review comments

r=me
Attachment #714347 - Flags: review?(philip.chee) → review+

Updated

4 years ago
Attachment #713193 - Flags: review?(iann_bugzilla)

Comment 14

4 years ago
Comment on attachment 714347 [details] [diff] [review]
Addressed review comments

r=me
Attachment #714347 - Flags: review+
(Assignee)

Comment 15

4 years ago
Pushed comm-central changeset 98476aa17e13.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Target Milestone: --- → seamonkey2.18

Comment 16

4 years ago
Tue Feb 19 2013 22:01:31
Error: ReferenceError: FillInHTMLTooltip is not defined
Source file: chrome://navigator/content/navigator.xul
Line: 1
 ----------
Tue Feb 19 2013 22:01:26
Error: ReferenceError: getBrowserURL is not defined
Source file: chrome://infolister/content/seamonkey-support.js
Line: 2
 ----------
Tue Feb 19 2013 22:01:26
Error: TypeError: popup is null
Source file: chrome://navigator/content/safeBrowsingOverlay.js
Line: 42
 ----------
Tue Feb 19 2013 22:01:26
Error: NS_ERROR_INVALID_POINTER: Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIRDFService.GetDataSourceBlocking]
Source file: chrome://communicator/content/sidebar/sidebarOverlay.js
Line: 709
 ----------
Tue Feb 19 2013 22:01:26
Error: ReferenceError: isElementVisible is not defined
Source file: chrome://navigator/content/navigator.js
Line: 709
 ----------
Tue Feb 19 2013 22:01:26
Error: error in processing external entity reference
Source file: jar:file:///C:/T1/hg/objdir-sm/mozilla/dist/seamonkey/omni.ja!/chrome/comm/content/navigator/urlbarBindings.xml
Line: 9, Column: 3
Source code:
  %textcontextDTD;
 ----------
Tue Feb 19 2013 22:01:26
Error: error in processing external entity reference
Source file: jar:file:///C:/T1/hg/objdir-sm/mozilla/dist/seamonkey/omni.ja!/chrome/comm/content/communicator/search/search.xml
Line: 10, Column: 3
Source code:
  %textcontextDTD;

Comment 17

4 years ago
IanN pointed me at: http://hg.mozilla.org/comm-central/rev/98476aa17e13#l8.12

    8.12 -<!-- LOCALIZATION NOTE : FILE This file contains the global menu items --> 
    8.13 +!-- LOCALIZATION NOTE : FILE This file contains the global menu items --> 

Bustage fix pushed r=me
http://hg.mozilla.org/comm-central/rev/04714c3e9acd

Updated

4 years ago
Depends on: 842529
(Assignee)

Updated

4 years ago
Depends on: 875889, 883577

Updated

4 years ago
Depends on: 891081
You need to log in before you can comment on or make changes to this bug.