Closed Bug 255196 Opened 18 years ago Closed 18 years ago

DDE: Request WWW_OpenURL send to Mozilla web page address without last character of address.

Categories

(SeaMonkey :: General, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: p.juracic, Assigned: neil)

References

Details

(Keywords: fixed-aviary1.0, fixed1.7.5)

Attachments

(3 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko/20040803
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.2) Gecko/20040803

When you want open web page for example 'www.google.com' by DDE Request call,
Mozilla open page 'www.google.co'.
Missing last character in web page address.

Try thi small VisualBasic macro for Excel:
Sub DDEcallMozilla()
    Dim tmp
    Dim channelNumber
    
        channelNumber = Application.DDEInitiate("Mozilla", "WWW_OpenURL")
        tmp = Application.DDERequest(channelNumber, "www.google.com")
        Application.DDETerminate channelNumber
        
End Sub

Sorry for M$ macro, If you want I can send you VI code in NationaInstrument
LabVIEW :-))

Reproducible: Always
Steps to Reproduce:
1. Close all Mozilla browser [and Download Manager]
2. Open first Mozilla browser 
3. Now try DDE Reguest to Server(application) 'Mozilla' with Topic 'WWW_OpenURL'
and with Item 'www.google.com'. Or run there Excel Macro


Actual Results:  
Mozilla open new browser window and start opening page 'www.google.co'
Missing last character in web page address.

Expected Results:  
Expect opening web address 'www.google.com'
Using that code, I did reproduce the behavior.  However, the message format used
when I do start->run "http://www.google.com" (which works properly) would be:
"%1",,-1,0,,,,

Using the following code, google loads properly:

Sub DDEcallMozilla()
    Dim tmp
    Dim channelNumber
    
        channelNumber = Application.DDEInitiate("Mozilla", "WWW_OpenURL")
        tmp = Application.DDERequest(channelNumber, "" & Chr$(34) &
"www.google.com" & Chr$(34) & ",,-1,0,,,,")
        Application.DDETerminate channelNumber
        
End Sub

I don't know if this is a Mozilla bug, or if you aren't sending the right message.
I understand www.google.com fails, and "www.google.com",,-1,0,,,, succeeds, but
how about "www.google.com" or www.google.com,,-1,0,,,, ? This well tell us if
it's the lack of the quotation marks or the commas that's confusing Mozilla.

Also, if you have a tree, try compiling nsNativeAppSupportWin.cpp with
-DMOZ_DEBUG_DDE to see where the URL is getting mangled.
"www.google.com" succeeded.
www.google.com,,-1,0,,,, also succeeded.
(In reply to comment #2)
> Also, if you have a tree, try compiling nsNativeAppSupportWin.cpp with
> -DMOZ_DEBUG_DDE to see where the URL is getting mangled.

I don't know how to do that.
Alternatively deleting lines 143 and 146 should work.
Attached file DDE Dump
This was the result of using the original code (www.google.com).
Attached patch Proposed patch (obsolete) — Splinter Review
Assignee: general → neil.parkwaycc.co.uk
Status: UNCONFIRMED → ASSIGNED
Comment on attachment 160075 [details] [diff] [review]
Proposed patch

DdeQueryString returns the string length, which is one less than the buffer
length it requires, to allow for the terminating null character.
Attachment #160075 - Flags: superreview?(rbs)
Attachment #160075 - Flags: review?(emaijala)
Comment on attachment 160075 [details] [diff] [review]
Proposed patch

Stealing more reviews ;)
Attachment #160075 - Flags: review?(emaijala) → review+
Comment on attachment 160075 [details] [diff] [review]
Proposed patch

I'm no string expert, but I believe the parameter to SetLength shouldn't
include the space for terminating null. Would temp.Length()+1 in the
DdeQueryString be more appropriate?
Attachment #160075 - Flags: review+ → review-
Attached patch Better patchSplinter Review
Attachment #160075 - Attachment is obsolete: true
Comment on attachment 160085 [details] [diff] [review]
Better patch

ere, thanks for catching that. Also, here's -u7 for you.
Attachment #160085 - Flags: superreview?(rbs)
Attachment #160085 - Flags: review?(emaijala)
Attachment #160085 - Flags: review?(emaijala) → review+
Comment on attachment 160085 [details] [diff] [review]
Better patch

sr=rbs, include temp.SetLength( argLen+1 ); as well...

I see some conflicting comments in the tree (see below). No point being greedy
on 1 char when crossing a M$ boundary.

void
nsTSubstring_CharT::SetLength( size_type length )
  {
    SetCapacity(length);
    mLength = length;
  }
----
void
nsTSubstring_CharT::SetCapacity( size_type capacity )
  {
    // capacity does not include room for the terminating null char
[...]
}
Attachment #160085 - Flags: superreview?(rbs) → superreview+
Attachment #160075 - Flags: superreview?(rbs)
Darin, please can settle this argument once and for all?
SetLength(N) ensures a buffer size that consists of N storage units plus one
extra storage unit for the nul-terminator.

It so happens that SetCapacity(M) is equivalent to SetLength(M).  In the string
code, the capacity of a string does not include the nul-terminator.  This may
seem counter-intuitive to those familiar with other string libraries.  Please
note that SetCapacity is not guaranteed to actually do anything.

The current patch is fine.  There is no need to increase the size of the string
buffer.
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
The patch looks easy and DDE requests are important to some people. Could it be
considered for branches ?
Flags: blocking1.7.x?
Flags: blocking-aviary1.0?
bug 266896 is the same for Firefox. Please check it out and resolve as duplicate
of this one it is so.
Comment on attachment 160085 [details] [diff] [review]
Better patch

If this patch is working for Firefox on the branch then we want it. Get it in
for tomorrow morning's builds.
Attachment #160085 - Flags: approval-aviary+
*** Bug 266896 has been marked as a duplicate of this bug. ***
Fixed on the aviary branch, a=asa.
Keywords: fixed-aviary1.0
Flags: blocking-aviary1.0?
Product: Browser → Seamonkey
Fixed on aviary, I think it should go on 1.7 branch too (same behaviour for
Mozilla 1.7 and Firefox)
Comment on attachment 160085 [details] [diff] [review]
Better patch

As per comment 23 requesting 1.7/aviary sync.
Attachment #160085 - Flags: approval1.7.5?
Comment on attachment 160085 [details] [diff] [review]
Better patch

a=mkaply for 1.7.5
Attachment #160085 - Flags: approval1.7.5? → approval1.7.5+
Keywords: fixed1.7.5
Flags: blocking1.7.5?
You need to log in before you can comment on or make changes to this bug.