52674 - Privacy&Security submenu and Cookies pref button need re-routing

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P1)

Description

[verah (gone)]

Apologies if this is the wrong component...

As per an agreement with Kent Walker in legal, these two UI elements:

Cookies Preferences dialog "More Info" button
Tasks:Privacy&Security:Understanding Privacy menu item

Should point to the online Help document titled, "Understanding Privacy and 
Security." This document contains the wording that meets with legal's 
requirements for informing users about cookies.

To open the file from the menu item and button, we need to use the same code (I 
believe it is Javascript) used in the Help menu (Help:Help 
Contents:Understanding Privacy and Security). Ian Oeschger can explain how to do 
this.

Comment 1

[johng]

nsbeta3, rtm keywords

This was what Steve, Vera and I agreed to do for legal and usability reasons.
The new doc will have a link to the current doc so this info will be prevalent.
 If there is no new information, we should not change our earlier decision.

Steve and Veditz, please nsbeta3+ and P1 or P2 this - it is an easy, low risk
change.

Comment 2

[Daniel Veditz [:dveditz]]

I thought another bug declared that the Privacy and Security document should be 
shipped with the product, to avoid the ironic potential privacy problems with 
hitting a Netscape server when reading about privacy?

Comment 3

[verah (gone)]

Yes, there is a bug like that, and it is related to this one, but this is a very 
specific bug asking to replace one document with another. The one being replaced 
is still part of the online help, and still needs to be moved from its server 
location to a local url. We're working on that, too. (Sorry I can't find the 
number of that one right now, but it's really late as you no doubt have noticed, 
and I'm really, really tired!)

Comment 4

[Daniel Veditz [:dveditz]]

Oh, OK. I thought this one was undoing the change of having the old one moved 
into the chrome. I guess this falls under the "legal requirement" heading so 
bumping to P1. If checked into the tree it constitutes a UI change, though, so 
get it in as soon as possible. May have to jump through hoops justifying the 
change with the translation folks.

Comment 5

[sairuh (rarely reading bugmail)]

over to cookies, as a guess...

Comment 6

[verah (gone)]

Since it's a help document, they're not expecting to translate till October 13. 
So we have a little time. The menu item, "Understanding Privacy," stays the same 
-- it just points to a different URL.

Comment 7

[Phil Peterson]

PDT agrees P1

Comment 8

[Chris McAfee]

what's the online help URL?
I can do the task menu part of this if needed.

Comment 9

[verah (gone)]

You can get the information on how to open the "Understanding Privacy" help file 
from Ian Oeschger. I'll let him know you're working on it.

Comment 10

[Ian Oeschger (gone)]

mcafee: check out the function in 
http://lxr.mcom.com/commercial/source/xpfe/communicator/resources/locale/en-US/h
elpMenu.rdf#98. That's us opening the Privacy help chrome with the privacy 
content in a subframe.

Comment 11

[Ian Oeschger (gone)]

toOpenWindowByType('help:security', 
'chrome://communicator/content/help/help_security.xul')

From our content, where we can't easily use this function and avoid multiple 
instances, we are just saying 
openDialog('chrome://communicator/content/help/help_security.xul'). Hope that 
helps.

Comment 12

[Stephen P. Morse]

Chris, I have this under control.  I know exactly where to plug it in and will 
do so just as soon as a URL is ready.  From the comments I've seen above, I 
still don't see a chrome url for the particular document.  The one URL cited, 
namely:

   chrome://communicator/content/help/help_security.xul

is not yet checked in as far as I can tell.

Comment 13

[Ian Oeschger (gone)]

It's there on Windows and Mac, and I think it's waiting for the jar xp packaging 
to get put into the Linux install. The XUL file loads the security content in 
chrome://communicator/locale/help/securitycont.html and its UI strings from 
chrome://communicator/locale/help/help_security.dtd

Here is the XUL file in the source:
http://lxr.mcom.com/commercial/source/tpubs/ns6/helpchrome/content/help_security
.xul

Comment 14

[Stephen P. Morse]

Attachment: Patch to fix the problem

Comment 15

[Stephen P. Morse]

Note, I already removed the diagnostic dump statements from the changes although 
they are still shown in the patch.

Comment 16

[Daniel Veditz [:dveditz]]

Steve: I think I'm the module owner for prefs backend now, so I give approval 
for the all.js change. My only suggestion as a reviewer would be that since the 
wallet privacy.html still exists, why not handle the exception in 
pref-cookies.xul by going ahead and showing the old document in the catch 
block.  r=dveditz

Comment 17

[Stephen P. Morse]

Attaching a revised set of diffs which contain the following changes:

1. Removed diagnostic dump statements as noted by ben
2. Use original tutorial if error occurs as suggested by dveditz
3. Need separate URLs if coming from menu or coming from prefs.

Comment 18

[Stephen P. Morse]

Attachment: revised patch

Comment 19

[Daniel Veditz [:dveditz]]

9/22 18:57 patch r=dveditz

Comment 20

[Ben Goodger (use ben at mozilla dot org for email)]

r||a=ben for the applicable sections. 

Comment 21

[Brendan Eich [:brendan]]

Minor comments:
- diff -u format next time please.
- use const to avoid repeating window options:
    function viewTutorial() 
    {
      const options = "modal=yes,chrome,resizable=yes,height=400,width=600";
!     try {
!       var pref = 
Components.classes["@mozilla.org/preferences;1"].getService();
!       if (pref)
!         pref = pref.QueryInterface( Components.interfaces.nsIPref );
!       window.openDialog(
!         pref.getLocalizedUnicharPref("wallet.TutorialFromPrefs"),
!         "", options, 0);
!     }
!     catch(e) {
!       window.openDialog
!         ("chrome://communicator/content/wallet/privacy.xul","",options, 0);
!     }
    }

(might want to use const for the contractid and the fallback chrome URL too, 
shorten long lines and group the constants together up top -- just a thought).

/be  

Comment 22

[Stephen P. Morse]

Attachment: Revised patch for pref-cookies.xul per brendan's comments

Comment 23

[Daniel Veditz [:dveditz]]

r=dveditz for the new patch

Comment 24

[matt]

r=matt

Comment 25

[verah (gone)]

r=verah for the patch to securitycont.html

Comment 26

[Stephen P. Morse]

Well it only took me five days and several follow-up messages, but I finally got 
all the reviews and approvals necessary to check in this change.  Namely:

a=dveditz for changes to prefs backend (all.js)
a=ben for changes to UI modules (navigator.properties, taskOverlay.js)
a=matt for changes to prefs front end (pref-cookies.xul)
r=dveditz for normal review of final patch attached to bug report
r=ben for super review of final patch attached to bug report

So I was about to check in but then I reread the new rules and realize that I 
can't check in on the branch until this bug receives a nsbeta3++ rating. :-(

Comment 27

[Stephen P. Morse]

Oh, I forgot to mention above:

a=verah for doc changes (securitycont.html)

Comment 28

[Blake Ross]

since none of those people are super reviewers, approval is also needed from 
someone in the list at http://www.mozilla.org/hacking/reviewers.html

if you think it should be ++, appeal to pdt ([e-mail])

Comment 29

[Daniel Veditz [:dveditz]]

approving as rtm+ candidate to get this on PDT radar for rtm++ approval.

Comment 30

[Stephen P. Morse]

Blake, ben is the super reviewer for this

Dan, according to the rules on the tinderbox branch page, I need to get an 
nsbeta3++ rating, not an rtm++

Comment 31

[Stephen P. Morse]

OK, I now understand the difference between nsbeta3++ and rtm++ (I'm a little 
slow when it comes to these things) and will focus on rtm and forget about 
beta3 for this change.  Also dveditz just caught the fact that we also need l10n 
approvals because property-file changes are involved.  I'll start seeking this 
out immediately.

Comment 32

[Michele Carlson]

This is fine to fix after PR3 ships, for RTM as noted above. L10n approves this 
fix for RTM.

Comment 33

[Daniel Veditz [:dveditz]]

Out of PR3, into early RTM when the L10n impact is lower. PDT: Please approve,
this was a request from Legal.

Comment 34

[Daniel Veditz [:dveditz]]

PDT: please approve rtm++ -- we have all the permissions and reviews including
l10n permission to make this change, but obviously it has to go in soon due to
translation impact.

Comment 35

[Phil Peterson]

PDT marking [rtm++]

Comment 36

[Stephen P. Morse]

Checked in to both the branch and the trunk.

Comment 37

[gerardok]

tever please verify

Comment 38

[Tom Everingham]

Verified on branch:
WinNT 2000100908
Linux 2000100909
Mac 2000100910

needs verified on trunk

Comment 39

[Rodney Velasco]

verified on trunk
tested on
Oct. 19 Linux Trunk
Oct. 23 Windows Trunk
Oct. 23 Mac Trunk
Removing vtrunk keyword and marking as verified