Modify the Palm Installer to search for Palm Dlls on local disk

VERIFIED FIXED in mozilla1.2beta

Status

SeaMonkey
Installer
P2
blocker
VERIFIED FIXED
16 years ago
14 years ago

People

(Reporter: Rajiv Dayal, Assigned: Rajiv Dayal)

Tracking

Trunk
mozilla1.2beta
x86
Windows NT

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

16 years ago
There are two binary files (CondMgr.dll and HsApi.dll), required by the Palm
Conduit Installer, which are provided by Palm and Palm provides free
redistribution license for them. However since these binaries are not "open
source" it is advisable to rather not use them for redistribution as part of
Mozilla. 

The install code loads redistributable copies of these binaries to find the Palm
software (HotSync stuff) path and then install the palm conduit, the right thing
to do as per Palm documentation. But we need to change this.

If the user has a Palm handheld and installs the HotSync stuff that comes with
it, it is very likely that these binaries will exist on the users machine.
I have found a Registry key that seems to exist on machines where the Palm
software is installed. I have changed the code to use this registry key to find
the Palm software path and load these 2 dlls from the Palm software location
thus avoiding the need to redistribute them.
(Assignee)

Comment 1

16 years ago
Also the PalmSync support module is not yet added in the installer manifest and
registration of Mozilla Palm sync support proxy is currently needed to be done
by hand. So the first needs to be done and the Conduit Installer needs to do
registration and unregistration of support proxy during conduit install and
un-install.
Severity: normal → critical
Priority: -- → P2
Target Milestone: --- → mozilla1.2beta
(Assignee)

Comment 2

16 years ago
Created attachment 102528 [details] [diff] [review]
patch to take care of all 3 issues above

patch takes care of:
1. Tries to locate Palm binaries(CondMgr.dll, HsApi.dll) using registry key
setting
2. Make palmsync support module part of install manifest
3. Do registration of palmsync support module from the Conduit Installer.
(Assignee)

Comment 3

16 years ago
Sean can u please review this patch above.
(Assignee)

Comment 4

16 years ago
Changing priority to blocker since without this Palm Sync cannot be used.
Severity: critical → blocker

Comment 5

16 years ago
Comment on attachment 102528 [details] [diff] [review]
patch to take care of all 3 issues above

r=ssu
Attachment #102528 - Flags: review+
(Assignee)

Comment 6

16 years ago
Hi Dan,

Can u please super review this bug, thanks.
Comment on attachment 102528 [details] [diff] [review]
patch to take care of all 3 issues above

>     // Construct the path of the Palm Desktop Conduit Manager
>     TCHAR   szPDCondMgrPath[_MAX_PATH];
>+    strncpy(szPDCondMgrPath, szPalmDesktopDirectory, _MAX_PATH);

In general it's better to use sizeof(<buffer>) when possible to avoid changing
one definition and forgetting to catch the others. Note that if
szPalmDesktopDirectory was somehow larger than the _MAX_PATH value you pass in
the resulting string would not be null terminated.

>     strncat(szPDCondMgrPath, DIRECTORY_SEPARATOR_STR, _MAX_PATH-strlen(szPDCondMgrPath));
>     strncat(szPDCondMgrPath, CONDMGR_FILENAME, _MAX_PATH-strlen(szPDCondMgrPath));

Similar issues here. Maybe you could do _snprintf("%s%s%s",_MAX_PATH, ...)
instead. Check for a negative return value to see if you overflowed, or just
always slap a null in the last character of the buffer.

>+    if( (*hCondMgrDll=LoadLibrary(szPDCondMgrPath)) != NULL )
>+        // Successfully loaded CondMgr Library from Palm Desktop Directory
>     return 0;
>+
>+    return IDS_ERR_LOADING_CONDMGR;

The indentation here is confusing. I assume return 0; should be indented?

>+    if( (hMozPalmSyncProxyDll=LoadLibrary("PalmSyncProxy.dll")) != NULL ) {

Could you use a full path here? It's a pretty generic name, I'd hate to pick up
the wrong one from the windows path if ours wasn't installed for some reason.
(Assignee)

Comment 8

16 years ago
Created attachment 102631 [details] [diff] [review]
updated patch to take care of sr comments

Hi Dan, Can you please sr this patch, thanks.
Attachment #102528 - Attachment is obsolete: true
Comment on attachment 102631 [details] [diff] [review]
updated patch to take care of sr comments

sr=dveditz
Attachment #102631 - Flags: superreview+
Attachment #102631 - Flags: review+

Comment 10

16 years ago
Comment on attachment 102631 [details] [diff] [review]
updated patch to take care of sr comments

a=asa for checkin to 1.2beta (on behalf of drivers)
Attachment #102631 - Flags: approval+
(Assignee)

Updated

16 years ago
Blocks: 155417
(Assignee)

Comment 11

16 years ago
fix has already landed, marking as fixed.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 12

16 years ago
Verified code fix. Rajiv, I'm assuming it's working as desired. ???
Status: RESOLVED → VERIFIED
QA Contact: bugzilla → ktrina
Why is this "Verified code fix"? this should be testable by anyone able to test
the Palm feature in the first place.

Comment 14

16 years ago
Well, then can someone with a Palm give this a go?
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.