366245 - SeaMonkey doesn't write dde server keys to the Windows Registry

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Carlos Une]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20061231 SeaMonkey/1.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20061231 SeaMonkey/1.1

After setting up SeaMonkey as default browser, SeaMonkey doesn't write dde server keys to the Windows Registry.

Reproducible: Always

Steps to Reproduce:
1. Edit -> Preferences -> Navigator -> Set Default Browser -> OK
2. Restart SeaMonkey
3. Open the Registry Editor, then navigate to HKEY_CLASSES_ROOT\HTTP\shell\open
Actual Results:  
'ddeexec', 'ddeexec\application' and 'ddeexec\topic' subkeys don't exist

Expected Results:  
'ddeexec', 'ddeexec\application' and 'ddeexec\topic' subkeys should point to SeaMonkey

SeaMonkey writes values in HKEY_CLASSES_ROOT root directory.

Comment 1

[Carlos Une]

Attachment: patch for SM-1.1

Comment 2

[Benoît]

Attachment: snapshot of what SeaMonkey currently does

To clarify, instead of adding the keys at the correct place, it adds them under the top-level classes key.

Comment 3

[[e-mail]]

Comment on attachment 250767 [details] [diff] [review]
patch for SM-1.1

Over to CTho as KaiRo blames him for this code (in bug 287943 comment #7).

Note: you don't seem to close your keys anywhere.

Comment 4

[Carlos Une]

Attachment: closing keys

Comment 5

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Comment on attachment 250790 [details] [diff] [review]
closing keys

Use spaces instead of tabs in patches.

I'm not sure I'm qualified to review this... if you want me to do the review you'll need to give me time to read up on the API on MSDN.  I don't even remember doing the patch KaiRo referenced.  I also don't understand our DDE code (any more) so I'd have to (re)learn that too.

Does the bug exist in Toolkit?

Comment 6

[[e-mail]]

Comment on attachment 250790 [details] [diff] [review]
closing keys

Maybe ere can spare the time to look at this fix.

Comment 7

[Frank Wein [:mcsmurf]]

Just FYI: Bug 353089 contains some info about what FF does and what changed a few month ago in their code.

Comment 8

[Ere Maijala (slow)]

Comment on attachment 250790 [details] [diff] [review]
closing keys

Sure, but it'll take me a while to get my Seamonkey build working again..

Comment 9

[Ere Maijala (slow)]

Comment on attachment 250790 [details] [diff] [review]
closing keys

Get rid of tabs and Windows  line-endings. With those changed, r=emaijala.

Comment 10

[[e-mail]]

Comment on attachment 250790 [details] [diff] [review]
closing keys

Don't worry about the line endings, MSYS patch doesn't seem to mind them.
The tabs do need fixing though.

>+		rc = ::RegCreateKeyEx(HKEY_CLASSES_ROOT,
>+				"http\\shell\\open\\ddeexec",
Please line up the parameters under each other.
If you think it looks better, you can list multiple parameters on one line,
but you should limit the lines to 80 characters in any case.

>+		
>+		if (REG_SUCCEEDED(rc)) {
>+			::RegSetValueEx( hKey,
>+				"",
>+				0,
>+				REG_SZ,
>+				ddeexec,
>+				sizeof ddeexec );
>+			::RegCloseKey(hKey);
>+		}
Please move the declaration of ddeexec into this block. (Similarly for topic). Also, you're not being consistent with your bracketing style; sometimes you're using ( mAppName ) and sometimes (hKey); please stick to one or the other.

>+				(unsigned char *) mAppName,
The parameter is a BYTE * is it not?

Comment 11

[Carlos Une]

Attachment: tabs, line endings, variable declaration, etc

Comment 12

[Carlos Une]

Attachment: patch

RegSetValueEx function expects a const BYTE* parameter.

Comment 13

[[e-mail]]

Comment on attachment 251719 [details] [diff] [review]
patch

sr=me

Comment 14

[Andrew Schultz]

Comment on attachment 251719 [details] [diff] [review]
patch

landed on trunk

Comment 15

[[e-mail]]

Comment on attachment 251719 [details] [diff] [review]
patch

I guess this needs to go on the branch too, right?

Comment 16

[Andrew Schultz]

Comment on attachment 251719 [details] [diff] [review]
patch

I landed this on the 1.8 branch