88123 - w/ browser running, doing 'edit' from windows explorer opens empty page

Status: VERIFIED FIXED

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

Description

[shrirang khanzode]

as the summary states...if I have a browser instance running and I use windows 
explorer to edit a html doc from my HD ( right click->Edit)...then what opens up 
in editor is the 'HTML Test Page' and not the actual document that I clicked 
on..

If no browser instance is running, then right clicking on a html file on my 
HD...launches the browser and opens up the html doc in composer

Comment 1

[rubydoo123]

what are you talking about?

Comment 2

[Paul Wyskoczka]

I see that same behavior running on win2k.  When I right mouse click using the
context menu and open an html file on my hard disk, the test page is displayed
in the composer.

Comment 3

[rubydoo123]

oh, I get it, you are right clicking on the desktop icon -- I see that too,
that's quite odd to get the test page

Comment 4

[rubydoo123]

*** Bug 88313 has been marked as a duplicate of this bug. ***

Comment 5

[rubydoo123]

handing over to command line engineer, this must be hard-coded somewhere

Comment 6

[rubydoo123]

*** Bug 92492 has been marked as a duplicate of this bug. ***

Comment 7

[sujay]

can we get a target milestone for this bug? 

Comment 8

[Paul Chen]

nav triage team;

Marking p3, mozilla0.9.4, reassigning to pchen, adding nsbeta1+

Comment 9

[Blake Ross]

This belongs to editor. See bug 93737.

Comment 10

[rubydoo123]

I'm going to dup 93737 to this older bug, but wanted to copy some info from that 
bug here:

When using Composer to open files from anywhere outside of Mozilla (i.e. from
win explorer), Editor just opens some Ender test page.  Luckily that's not too
embarrassing!

The real fix will be to let other apps pass in arguments, but for now (just to
remove loading the test page), the fix is to change value to about:blank or 
nothing:

http://lxr.mozilla.org/seamonkey/source/editor/ui/composer/content/editor.xul#10
5

looks like pav did that part of the code -- cc'ing him

Comment 11

[rubydoo123]

*** Bug 93737 has been marked as a duplicate of this bug. ***

Comment 12

[rubydoo123]

assigning this to brade to get some traction on it

Comment 13

[Charles Manske]

It's Windows only, so brade isn't the appropriate person

Comment 14

[Charles Manske]

This works correctly in my Netscape 6.1 installation:
There is a Windows registry entry to associate an application to "open" and 
"edit" HTML and HTM file types:
In the HKEY_CLASSES_ROOT section, we register our file association in the 
key = "MozillaHTML->shell->open->command" as 
N:\NETSCAPE\NEAC18~1\NETSCP6.EXE -url "%1"
(the path to your installed netscape program)
and for key = "MozillaHTML->shell->edit->command" as 
N:\NETSCAPE\NEAC18~1\NETSCP6.EXE -edit "%1"
If I right click on an HTML file in Windows file explorer, there is a context
menu option "edit", which loads the file into Composer correctly.

So is the issue that we aren't doing this in the mozilla build?
Maybe this registry stuff is only happening in the Netscape Commercial build?

Comment 15

[kmurray1115]

I can't vouch for the mozilla builds, but I do see this on Commercial. On Win2K,
in "Internet Options, there are three options for "HTML Editor:" 
"Netscape"
"Netscape Navigator"
"Notepad"
Using the first instance ("Netscape") should do what I'm asking it to do, in
fact that's what should happen when I choose to use N6 as my default internet
app, but setting the HTML Editor to "Netscape" then gets me to the Ender test
page when I use the context menu|edit command. Using the second instance -
"Netscape Navigator" opens 4.x Composer.

So, we're not registering ourselves as a "MAPI-like" option as the HTML editor
on Win2K. Can't vouch for other platforms.

Comment 16

[Blake Ross]

I see this in commercial, it happens when I go to Edit in MOZILLA (yes, it's a
commercial build) in IE's File menu.  And it makes sense that it's happening;
look at the code snippet I provided, that beppe cited.

Comment 17

[Kathleen :Brade]

Blake--why does it make sense that the editor not be opening the file you want to 
edit?

Comment 18

[Blake Ross]

Well, I don't know Editor that well -- how are external applications supposed to
pass in the url they want to load?

It makes to me from a limited code perspective because we load the value of args:

http://lxr.mozilla.org/seamonkey/source/editor/ui/composer/content/editor.js#214

and args defaults to the test page:

http://lxr.mozilla.org/seamonkey/source/editor/ui/composer/content/editor.xul#105

and I don't see how an outside application could pass an argument into
window.arguments to override that.

Comment 19

[Charles Manske]

As I said in my description of the Windows' registry entry, the filename is passed
in as the "%1" in the "edit" registry entry.
We should definitely change the default "args" value to be "about:blank", though,
and not the test page.
Can someone who has installed using the mozilla or NS commercial build please
show what they are seeing for the "MozillaHTML->shell->edit->command" entry in
their Windows registry?

Comment 20

[shrirang khanzode]

Charlie, I have the commercial 0806 trunk on my NT box:

The 'MozillaHTML->shell->edit->command' on my wndows registry shows the 
following:

"C:\TESTS\0806TR~1\NETSCP6.EXE-url "%1'"'

Comment 21

[Kathleen :Brade]

I checked in Blake's proposed fix (sr=kin)
This bug should now be "why don't we open the file instead of an empty window"

Comment 22

[rubydoo123]

I see this:
"C:\PROGRA~1\NETSCAPE\NETSCA~1\NETSCP6.EXE -edit "%1""

Comment 23

[rubydoo123]

moving to 1.0

Comment 24

[Charles Manske]

*** Bug 96462 has been marked as a duplicate of this bug. ***

Comment 25

[sujay]

*** Bug 97812 has been marked as a duplicate of this bug. ***

Comment 26

[sujay]

we're getting lot of DUPS...marking QAHP

Comment 27

[Kathleen :Brade]

*** Bug 98501 has been marked as a duplicate of this bug. ***

Comment 28

[Kathleen :Brade]

*** Bug 91967 has been marked as a duplicate of this bug. ***

Comment 29

[Kathleen :Brade]

*** Bug 99359 has been marked as a duplicate of this bug. ***

Comment 30

[Syd Logan]

spam composer change

Comment 31

[Charles Manske]

This is a module startup or DDE issue. Discussion with Bill Law reveals that
there are a few annoying problems with launching apps on Windows platforms,
using DDE to launch windows, differences in different Windows versions, etc.

Comment 32

[Kathleen :Brade]

After the request for publishing functionality, this is the most referenced bug 
for Composer in the netscape feedback newsgroups.  Is there any way to get this 
fixed before mozilla 1.0?

Comment 33

[Bill Law]

I'll look into this at least.  Not sure about fixing it yet.

Comment 34

[sujay]

cc: curt....Curt, you mentioned you were experiencing this problem also...

Comment 35

[sujay]

Marking nsbeta1+ per syd. Need to fix for composer usability.

Comment 36

[Kathleen :Brade]

*** Bug 109351 has been marked as a duplicate of this bug. ***

Comment 37

[Bill Law]

Resetting target milestone for all "window integration" bugs to mozilla0.9.8.  
I'm working on performance and won't get to that till next milestone.

Comment 38

[Kathleen :Brade]

*** Bug 113422 has been marked as a duplicate of this bug. ***

Comment 39

[Dimitrios]

*** Bug 114657 has been marked as a duplicate of this bug. ***

Comment 40

[Bill Law]

Attachment: Fix for this bug and some others (101025 and 99321, maybe more)

The problem is that we weren't doing the hard command line handling when
requests came in from secondary processes.  I've fixed that by reusing the code
already in nsAppRunner.cpp that handles initial startup.

Notes for reviewers:

nsCommandLineService.cpp:

1. Need to make copy of argv array because HandleRequest has no means
   of freeing it otherwise.  This was actually a lurking bug that
   was never hit because nobody else was calling GetArgv on the
   cmdlineservice it built.  Likewise, we free the copy in the dtor.

nsAppRunner.cpp:

1. DoCommandLines needs to tell HandleRequest whether it opened a
   window or not so a default Nav window can be opened if one wasn't
   (like Ensure1Window does in nsAppRunner.cpp itself).  So a PRBool
   result was added to the LaunchApp functions, the pref enumerator
   "closure" struct, and to DoCommandLines.  Almost every change in
   this file was to add support for returning that state indicator.

2. I removed the assertion in HandleArbitraryStartup.  Previously,
   the only caller to DoCommandLines always ensured "heedStartupPrefs"
   was true if there were no command line args.  But that's not the
   case from HandleRequest (in cases where there are no command lines
   but windows are already open).  Rather than avoid calling
   DoCommandLines in this case, I removed the assertion.  The code
   subsequent to the assertion is fine regardless (it keys off argc
   anyway) and this will permit us the flexibility to move the
   Ensure1Window-like logic into DoCommandLine at some point.

nsNativeAppSupport.h:

1. Added extern declaration for DoCommandLines.

nsNativeAppSupportWin.cpp:

1. I changed the code for TURBO_NAVIGATOR to ask use "-browser"
   (otherwise, choosing Navigator used the startup prefs).

2. In the WM_LBUTTONDBLCLICK code, I removed the check for open
   windows (it moved to HandleRequest.	Note that this means
   double-clicking always opens at least one window, per bug
   101025.

3. Removed code that called GetStartupURL.

4. Inserted (after handling "-kill") code that:
   a. Ensures a profile (moved from further down; this needs
      to happen before trying to open a window).
   b. Calculates whether we need to pay attention to startup
      prefs.  Commented sufficiently, I hope.
   c. Calls DoCommandLines and returns if it opens a window.

5. Removes GetStartupURL() which is no longer used.

Comment 41

[Dimitrios]

*** Bug 117430 has been marked as a duplicate of this bug. ***

Comment 42

[Samir Gehani]

Comment on attachment 63412 [details] [diff] [review]
Fix for this bug and some others (101025 and 99321, maybe more)

r=sgehani

Comment 43

[Kathleen :Brade]

*** Bug 119998 has been marked as a duplicate of this bug. ***

Comment 44

[sairuh (rarely reading bugmail)]

*** Bug 99814 has been marked as a duplicate of this bug. ***

Comment 45

[Blake Ross]

Comment on attachment 63412 [details] [diff] [review]
Fix for this bug and some others (101025 and 99321, maybe more)

sr=blake

Looks good. There are some 2 space/4 space indentation inconsistencies that
should be fixed.

Comment 46

[Bill Law]

fixed

Comment 47

[sairuh (rarely reading bugmail)]

verified fixed using 2002.01.29.10 comm verif bits on win2k --after making sure
that n6.x was set to handle html docs [checked it on in the Adv > System pref
panel].