Last Comment Bug 841230 - Add Open Link in Private Window context menuitem
: Add Open Link in Private Window context menuitem
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: unspecified
: All All
: -- normal (vote)
: seamonkey2.18
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on: 837510 856587 860905
Blocks: 460895
  Show dependency treegraph
 
Reported: 2013-02-13 16:43 PST by neil@parkwaycc.co.uk
Modified: 2013-04-11 15:39 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (5.08 KB, patch)
2013-02-14 00:46 PST, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Revised patch (6.00 KB, patch)
2013-02-15 06:20 PST, neil@parkwaycc.co.uk
iann_bugzilla: review+
philip.chee: review+
Details | Diff | Splinter Review

Description neil@parkwaycc.co.uk 2013-02-13 16:43:06 PST
Also hide the "Open Link in New Window" menuitem in private windows.
Comment 1 neil@parkwaycc.co.uk 2013-02-14 00:46:08 PST
Created attachment 713827 [details] [diff] [review]
Proposed patch
Comment 2 Philip Chee 2013-02-14 05:41:18 PST
Comment on attachment 713827 [details] [diff] [review]
Proposed patch

Thu Feb 14 2013 21:37:06
Error: ReferenceError: openNewPrivateWith is not defined
Source file: chrome://communicator/content/nsContextMenu.js
Line: 755
Comment 3 neil@parkwaycc.co.uk 2013-02-14 05:52:45 PST
Comment on attachment 713827 [details] [diff] [review]
Proposed patch

Please apply patches from unresolved dependent bugs before reviewing, thanks!
Comment 4 Philip Chee 2013-02-14 09:45:16 PST
Comment on attachment 713827 [details] [diff] [review]
Proposed patch

> -                oncommand="gContextMenu.openLink();"/>
> +                oncommand="gContextMenu.openLinkInWindow();"/>
> +      <menuitem id="context-openlinkprivate"
> +                label="&openLinkCmdPrivate.label;"
> +                accesskey="&openLinkCmdPrivate.accesskey;"
This should probably be:
openLinkInPrivateWindowCmd.label and openLinkInPrivateWindowCmd.accesskey .

> +                oncommand="gContextMenu.openLinkInPrivate();"/>
gContextMenu.openLinkInPrivateWindow()

> +  // Open linked-to URL in a private window.
> +  openLinkInPrivate: function() {
openLinkInPrivateWindow:

> +<!ENTITY openLinkCmdPrivate.label     "Open Link in Private Window">
> +<!ENTITY openLinkCmdPrivate.accesskey "d">
This access key conflicts with "Sen_d_ This Page".
Comment 5 neil@parkwaycc.co.uk 2013-02-14 13:02:46 PST
(In reply to Philip Chee from comment #4)
> openLinkInPrivateWindowCmd.label and openLinkInPrivateWindowCmd.accesskey .
Far too long.

> > +                oncommand="gContextMenu.openLinkInPrivate();"/>
> gContextMenu.openLinkInPrivateWindow()
Actually I see I should have called it openLinkPrivate().

> > +<!ENTITY openLinkCmdPrivate.accesskey "d">
> This access key conflicts with "Sen_d_ This Page".
Bah, I thought I'd checked everything :-(
Comment 6 neil@parkwaycc.co.uk 2013-02-14 13:03:43 PST
(In reply to comment #5)
> (In reply to Philip Chee from comment #4)
> > > +<!ENTITY openLinkCmdPrivate.accesskey "d">
> > This access key conflicts with "Sen_d_ This Page".
> Bah, I thought I'd checked everything :-(
Oh, different overlay, no wonder I overlooked it...
Comment 7 neil@parkwaycc.co.uk 2013-02-14 14:38:00 PST
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to Philip Chee from comment #4)
> > > > +<!ENTITY openLinkCmdPrivate.accesskey "d">
> > > This access key conflicts with "Sen_d_ This Page".
> > Bah, I thought I'd checked everything :-(
> Oh, different overlay, no wonder I overlooked it...
And of course my actual testing had been done with a linked image, on which Send This Page is already hidden. Any objections to hiding it for all links?
Comment 8 Philip Chee 2013-02-15 05:36:04 PST
>> gContextMenu.openLinkInPrivateWindow()
> Actually I see I should have called it openLinkPrivate().
OK you can keep the shorter names.

>>> Bah, I thought I'd checked everything :-(
>> Oh, different overlay, no wonder I overlooked it...
> And of course my actual testing had been done with a linked image, on which Send This
> Page is already hidden. Any objections to hiding it for all links?
No objections, sounds reasonable to me.
Comment 9 neil@parkwaycc.co.uk 2013-02-15 06:20:47 PST
Created attachment 714348 [details] [diff] [review]
Revised patch
Comment 10 Philip Chee 2013-02-15 08:53:14 PST
I think "O" is still available.
Comment 11 neil@parkwaycc.co.uk 2013-02-15 08:58:14 PST
(In reply to Philip Chee from comment #10)
> I think "O" is still available.
Used to block a popup, if you allowed it by mistake.
Comment 12 Philip Chee 2013-02-18 10:44:35 PST
Comment on attachment 714348 [details] [diff] [review]
Revised patch

Looks good. r=me
Comment 13 neil@parkwaycc.co.uk 2013-02-18 16:32:46 PST
Pushed comm-central changeset 447443b87c8e.

Note You need to log in before you can comment on or make changes to this bug.