89166 - *nsBranch only* dialog for File|Exit and x-close window when in -turbo mode

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P1)

Description

[Cathleen]

Comment 1

[timeless]

no description means you're asking for someone to provide a spec, which is what 
UID does best.

Comment 2

[lchiang]

cc: rudman.  Steve - can you provide the wording for this dialog?  cc: mcarlson
for localization.  I'd suggest assigning this bug to rudman for wording and then
vishy for implementation.

Cathleen/vishy - did we want this for nsbranch?  I thought we did.  If so, pls
mark this.  Thanks.

Comment 3

[rudman]

Ccing Jatin. He's already provided input for wording on the feature, so any
additional wording would be best if it came from him.

Comment 4

[Viswanath Ramachandran]

we will take this dialog on only the Netscape branch. This is not for the 
mozilla trunk or a long term solution. Blake is coding up a fix for this for the 
Netscape branch. 

Comment 5

[Jatin Billimoria]

I need more information about how this patch entails. Cathleen, you mentioned in
an email "that user passwords, email, aim are still been active if you stay with
-turbo mode running in the background." Can you elaborate on this? And how is
this bug affected by Bug 89165 ? 

Comment 6

[Jatin Billimoria]

CORRECTION: I need more information about *what* this patch entails.

Comment 7

[Blake Ross]

Attachment: patch

Comment 8

[Blake Ross]

Attachment: correct fix

Comment 9

[Jatin Billimoria]

Here is my suggested wording for Blake's patch:

"Quick Launch is active, so Netscape remains in memory and can be started again
quickly. You are still connected to mail accounts, instant messenging, and web
sites.  To completely exit Netscape, right click on the Netscape icon in the
bottom right of the taskbar and select Exit."

For the title bar, my suggestion:
+<!ENTITY exitWarningTitle.label "Quick Launch">

Comment 10

[Jatin Billimoria]

CORRECTION: "messenging" should be "messaging"

Comment 11

[Jatin Billimoria]

CORRECTION: Remove the extra space so it reads, "web sites. To completely"

Comment 12

[Blake Ross]

I like it. Clearer and less wordy than the current text.

* I think users are going to wonder when we say they're connected to websites 
(they're not, just some cookies might be hanging around).  Still, I can't think 
of any non-technical and accurate explanation...

* I don't think we have to go into so much detail about where the icon is, 
because the dialog is supplemented with a picture (I'll post a screenshot).  
Also, although they could have hidden the clock, most users won't, and I think 
it's an easier way to refer to that area than trying to describe it 
geographically (plus, if the taskbar is on the left or right side, it won't be 
in the bottom right...but that's an even more obscure case than the hidden 
clock, I suppose).  The "near the clock" text was inspired by MSN Messenger's 
dialog.

* Will most users understand that there's an implied "if you were connected" 
about the instant messaging, mail accounts, etc.?  i.e. they'll only still be 
logged in to NIM if they already were. I guess that's obvious.

Comment 13

[Blake Ross]

Is "in memory" non-technical enough that the average user is going to know what 
it means, or care?  MSN Messenger doesn't give so much information, nor do other 
apps I've seen (where such info appears, it's typically in the help file).

Comment 14

[Blake Ross]

Attachment: screenshot

Comment 15

[Jatin Billimoria]

How about this:

"Quick Launch is active, so Netscape stays in memory and can be started again
quickly. You are still logged in to mail accounts, instant messaging, and web
sites. To completely exit Netscape, right click on the taskbar icon (near the
time) and select Exit."

Comment 16

[Conrad Carlen (not reading bugmail)]

Index: resources/locale/en-US/profileSelection.dtd
===================================================================
RCS file: /cvsroot/mozilla/profile/resources/locale/en-US/profileSelection.dtd,v

Why are these resources being put into profile mgr's resources? What on earth
does this dialog have to do with profiles? Looks fine but I really, really think
it should go elsewhere in the tree.

+       nsCOMPtr<nsIDOMWindowInternal> domWin = do_GetInterface(docShell);

I'd check that domWin != nsnull before using it.

Comment 17

[Blake Ross]

I was trying not to add another file since this is only going on the branch.  
Also, the Exit item in the QuickLaunch context menu is already in a profile 
resource...But if you want, I'll add the file.

If domWin is null, our only choice is an unparented alert anyways, no?  So why 
check?

Comment 18

[Conrad Carlen (not reading bugmail)]

If it's not a problem to make another file, it would be good - in case we want
this dialog down the road on the trunk. For now, since it's only the branch, I'm
OK with it.

With the domWindow, the check would avoid a crash.
If the dialog was put up this way:

nsCOMPtr<nsIWindowWatcher> windowWatcher = do_GetService(...);
if (!windowWatcher) return NS_ERROR_FAILURE;
nsCOMPtr<nsIDOMWindowInternal> domWin = do_GetInterface(docShell);
windowWatcher->OpenWindow(domWin, ...)

and domWindow was NULL, we wouldn't crash - the dialog would be unparented.

Comment 19

[Blake Ross]

Er, yes, sorry...I was thinking of the old patch, which used windowwatcher and 
domWin as the parent.

New patch coming.

Comment 20

[Blake Ross]

Attachment: updated patch

Comment 21

[Conrad Carlen (not reading bugmail)]

A small part of the last patch is missing - confirmed with Blake. Assuming that
bit goes in, r=ccarlen

Comment 22

[timeless]

Arg.

jatin a few warnings: 1. Time might or might not be visible. On w2k terminal 
sessions iirc it is not by default. 2. What happens if a users runs -turbo and 
doesn't have an explorer.exe compatible shell (or it crashes)? 

If mozilla does not support the explorer notice (comes from an ie4 integration 
or newer explorer.exe) alerting all tray apps to reregister then when exploer 
restarts (after it crashes) users will be unable to quit netscape, this is a 
serious concern. it takes <5 lines to add this support but it must be done. 
risk: 0.1%.

The following is a rant, if someone feels i put it in the wrong bug please 
indicate the correct bug, thank you for your patience.

There _must_ be a way for me to safely Quit (#define Quit 
NS_LITERAL_STRING("unload Application; Leaving No Instances Or Traces In 
Memory") this application w/o using the tray.

Comment 23

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

box? what is this box?

sr=[e-mail]

Comment 24

[lchiang]

Update ETA per IM w/blake.  Will land when tree opens.

Comment 25

[Blake Ross]

Fixed on branch.

Comment 26

[Conrad Carlen (not reading bugmail)]

On a branch just pulled and built, this is doing something bad: When the dialog
comes up, it's empty, fills the screen, and I have to exit using the task
manager. Is anybody else seeing this?

Comment 27

[Blake Ross]

Oops, missed a line in my checkin; just checked it in. Thanks for catching that. 
update xpfe/browser/

Comment 28

[Paul Wyskoczka]

Verified in build 2001071006.

Comment 29

[Blake Ross]

Needs to go on trunk.

Comment 30

[lchiang]

blake - can you correct target milestone to 0.9.4 or later then?

Comment 31

[Paul Chen]

nav triage team:

Marking nsbeta1+ and p1, cc-ing pchen

Comment 32

[Jaime Rodriguez, Jr.]

PChen - Is Blake still working on this one? Is this a "stop-ship"? Will this one
be completed by today?

---> Pchen, because blaker has finished his internship, and now is blakeross, again.

Comment 33

[Blake Ross]

Yes, this is a stop ship, I'm working to juggle back-to-school and mozilla and
one is losing out. I will have it in by tonight. Please don't reassign my bugs,
even as blakeross, I'm still a real person.

Comment 34

[Conrad Carlen (not reading bugmail)]

Blake, you probably realized but just in case:
There's a better place to put this than was done back on the branch
http://lxr.mozilla.org/mozilla/source/xpfe/bootstrap/nsNativeAppSupportWin.cpp#1847
Here, the condition that the last window closing is the special window is
already dealt with.

Comment 35

[Jaime Rodriguez, Jr.]

sure thing blake . . . keep up the good work.

Comment 36

[Jaime Rodriguez, Jr.]

Cleaning Status Whiteboard.

Comment 37

[Blake Ross]

Attachment: patch

Comment 38

[Blake Ross]

- the showDialog pref defaults to true.
- disregard

+#include "nsIWindowsHooks.h"
+#include "nsIPromptService.h"

that's for another bug.

Comment 39

[Jaime Rodriguez, Jr.]

If Turbo is IN, we should nsbranch+ this bad boy, and request a PDT+.

Comment 40

[Blake Ross]

The fix is on the trunk...

Comment 41

[selmer (gone)]

Moving to nsbranch+, we still need this for the current release.

Comment 42

[selmer (gone)]

Is this fixed on the branch w/Law's checkin?

Comment 43

[Bill Law]

Yes, resolving as FIXED.

Comment 44

[Terri Preston]

Verified fixed w2k build 2001-10-03-05-0.9.4