83366 - -turbo mode: alternative 3-pane mail view setting is ignored

Status: RESOLVED FIXED

(Core Graveyard :: Cmd-line Features, defect, P2)

Description

[Dimitrios]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9+) Gecko/20010529
BuildID:    2001052920

In preferences I have selected alternative 3-pane Mail & News view. This works
fine unless I open Mail & News while Mozilla is resident (-turbo session). In
the same Windows session, after quitting Mozilla with File->Exit (resident part
is killed) and restarting it in the normal way (no -turbo), Mail & News view
restores Preferences setting.

Reproducible: Always
Steps to Reproduce:
1. If you hadn't already use alternative view, go to Prefs->Mail & Newsgroups
and set layout fot the second option (the one in the right)
2. Close Mozilla and restart with -turbo option.
3. Open Mail & News

Actual Results:  Mail & News main window is displayed in the default mode,
ignoring Preferences setting

Expected Results:  Mails & News main window should be displayed in the
alterative view, as set in Preferences

Comment 1

[Cathleen]

I am unable to confirm this bug...

I tried the following combinations:

- start mozilla
- go to Edit|Preference|Mail&News change layout to 2nd option
- File|Exit quit mozilla
- start mozilla with -turbo
- open Mail&News -> changes are applied
- File|Exit quit mozilla

- start mozilla with -turbo
- go to Edit|Preference|Mail&News change layout to 1st (different) option
- File|Exit quit mozilla
- start mozilla with -turbo
- open Mail&News -> changes are applied
- File|Exit quit mozilla

- start mozilla with -turbo
- go to Edit|Preference|Mail&News change layout to 2nd (different) option
- close mozilla window (leave app running in -turbo mode)
- start mozilla
- open Mail&News -> changes are applied
- File|Exit quit mozilla

The only thing I can think of is this may be an issue on win98 only.  My windows 
box is win2000.

Can anyone else comfirm?

Comment 2

[Cathleen]

law said that he was able to confirm this bug in last performance meeting.  :-)

Comment 3

[Viswanath Ramachandran]

nav +pdt triage: nsbeta1+, P2. 

Comment 4

[Bill Law]

Yes, I did say I had verified it.  But I was full of shit.

I mistook this for a different bug.  It was bug 83569 that I confirmed.  I don't
think this bug is real, or if it is real, has anything to do with -turbo mode.

Comment 5

[Dimitrios]

Still seing it on Win98, build 2001060420. Also seeing it on a different
(corporate pc) machine running Win95 OSR2, same Moz build. In the above cases,
both Mozillas are configured to show alternative view but return to the default
view on -turbo mode of operation. Both systems migrated from previous NS4
installations, if that helps. I 'll try to test a Win2k system to see what happens.

Comment 6

[Bill Law]

I think I understand what's going on.  This isn't a "turbo mode" problem, per 
se.  The problem is that the code that fields the request to open mail news 
*when Mozilla is already running* does not use the proper chrome URL to get the 
user-selected mailnews window.  It just uses chrome://messenger/content.

The code in question is in nsNativeAppSupportWin::HandleRequest where it looks 
for the "-mail" option.  At the time I wrote it, the logic that determined the 
chrome URL was elsewhere in nsAppRunner.cpp.  Now, I believe that code is in 
the mailnews "cmd line handler" interface implementation and we can get at it 
by using the category manager to find that interface and query it for the 
chrome url.

Alec, Vishy suggests you take over this bug.  I know you're an expert on the 
cmd-line handler stuff so you can probably fix this easier than I can, anyway.  
Probably we can fix the similar problem with "-edit" and add support so it 
supports other things like "-aim," etc.  Basically, the same code that's still 
in nsAppRunner.cpp.

Comment 7

[Alec Flett]

ah yes, I think I know what to do here - might be a little tricky, but basically
I need to ask the -mail command line handler for the chromeUrlForTask.

Comment 8

[Alec Flett]

Attachment: part one: add new API to nsICommandLineService

Comment 9

[Alec Flett]

Attachment: update bootstrap and native app support to use new API

Comment 10

[Alec Flett]

adding sspitzer for r=/sr=, since he knows this commandline handler stuff the
best...

ok, here's what these patches do:
- adds a new API to nsICmdLineService, getHandlerForParam() which allows you to
get the nsICmdLineHandler associated with a given parameter - does the work of
constructing a ProgID, etc
- adds some documentation to nsICmdLineService, and appropriately labels stuff
[noscript] so scripts don't go whacking memory they don't own
- getHandlerForParam will go through all of the parsed parameters, and keep
trying to find a handler for the given command, and returns the first one it
finds. (they are stored in command-line order, so if I say mozilla -mail -edit,
-mail will be the handler returned)
- updates nsAppRunner to use this mechanism, so we aren't duplicating code
- updates nsNativeAppSupport to try to handle arbitrary command lines, instead
of hardcoding -mail and -edit
- reorganizes some of the nsNativeAppSupportWin stuff to more cleanly show the
flow of control

With this patch, not only will alternate 3-pane work, but -chat, -xmlterm, and
all future mozilla apps will work through DDE.

Comment 11

[Blake Ross]

Looks good to me. r=blake

Comment 12

[jag (Peter Annema)]

File nsCommandLineService.cpp:

+  int i;
+  for (i=0; i< paramList->Count(); i++) {

Shouldn't that be a PRUint32?

+    // skip past leading / and -
+    while ((*param == '-') ||
+           (*param == '/')) param++;

So this skips past -, --, ---, etc., /, //, ///, etc., -/-/-/-, etc.
Is this desired behaviour?

Why not something like:

if (*param == '-' || *param == '/') {
  ++param;
  if (*param == *(param-1))
    ++param;
}


+    printf("Looking for contractID %s\n", contractID.get());

Comment out, remove, or put inside a #ifdef DEBUG_alecf

Comment 13

[Alec Flett]

Attachment: updated patch of command line API to address jag's comments

Comment 14

[(not reading, please use [e-mail] instead)]

r=sspitzer, looks good to me.

make sure to test really well.  all the profile related ones, and then the ones
that can take arguments, like -chrome, -edit etc.

for testing:  let's come up with a list of a bunch of cases that we should run
through before landing changes to this code.

I'll start it and put it on www.mozilla.org.

It seems like cmd line arg bugs fall on to sarah.  sarah, does a document like
this already exist?

Comment 15

[sairuh (rarely reading bugmail)]

test cases are at:

http://www.mozilla.org/quality/browser/front-end/testcases/cmd-line/

test plan is at:

http://www.mozilla.org/quality/browser/front-end/testplans/browser-xpapps/cmd-line.html

warning: haven't been updated for some time (just so's ya know :)

Comment 16

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 17

[Alec Flett]

argh, I'm without a windows box today, so I'm going to land this on 6/14

Comment 18

[Alec Flett]

ok, I still don't have a windows box, so I've now landed everything except for
nsNativeAppSupportWin.cpp - I'll check that in on 6/15

Comment 19

[Alec Flett]

yay, fix finally in

Comment 20

[Paul Wyskoczka]

I going to reopen this bug.  The first time you launch mail in turbo mode, the
default view appears.  In subsequent mail launches the alternative view appears.

Steps:
1. In the perference panel set the view to alternative (Right side view) and
exit the browser.
2. Start the browser in -turbo mode.
3. Start mail.  It comes up in the default view (Left side view). Close mail.
4. Relaunch mail. It comes up in the alternative view. 

This occurs each time the setting is changed to the alternative view.

Comment 21

[Cathleen]

reassign to vishy... alecf is on sabbatical till 9/3.

vishy, this is a regression... can you get someone to work on it??  -thanks!

Comment 22

[Viswanath Ramachandran]

-> blaker, could you take a look Blake? 

Comment 23

[Paul Wyskoczka]

I retested this using today's build 2001071006 and it looks ok.  I'm thinking I
might have used a trunk build when I tested yesterday. My bad.

Comment 24

[Blake Ross]

Paul: still, shouldn't this be fixed on the trunk as well?

Comment 25

[Viswanath Ramachandran]

Blake could you confirm that this is not a branch problem? 
If its trunk only, we shd keep it open but reduce its priority. 
If its branch, we shd go full court press to find the fix. 

Comment 26

[Blake Ross]

Paul said he was going to try a new build to confirm his sanity ;)

Comment 27

[Paul Wyskoczka]

I rechecked the trunk with the 7/10 build and I no longer see the problem. 
Looks good on both the branch and trunk.  OK to mark this bug resolved/fixed and
I will verify.

Comment 28

[Viswanath Ramachandran]

wfm

Comment 29

[Paul Wyskoczka]

Verified on Windows branch build 2001071006.

Comment 30

[timeless]

for my sanity i'm going to twiddle this bug.

Comment 31

[timeless]

alecf did the work

Comment 32

[timeless]

this was fixed 2001-06-15 15:00:13