Scott MacGregor Reporter
	Description • 19 years ago

some minor tweaks are needed to facilitate building the palm sync extension.

	Scott MacGregor Reporter
	Comment 1 • 19 years ago

step one, make sure the palmsync dll doesn't get folded into the static package list.

	Wayne Mery (:wsmwk)
	Comment 2 • 19 years ago

how will this "facilitate building the palm sync extension"?

	Scott MacGregor Reporter
	Comment 3 • 19 years ago

Changes to make us build an install.rdf based extension for palm sync, and to automate the building of the extension.

1) The build system automatically generates a palmsync-*version*.xpi file in dist\xpi-stage now. That file can be hosted on addons.mozilla.org once we fix the rest of the issues listed below.

2) I added entries to the removed-files list so users who install Thunderbird 2.0 will get the old files living in the application directory cleaned up and deleted.

Problems:

1) We need to find a way to call PalmSyncInstall.exe after the extenion is installed
2) We need to find a way to call PalmSyncInstall.exe /u when the user trys to disable or uninstall the extension. 

I don't know how we can make those two things happen with the extension manager.

3) Palm desktop is old! They have a 110 character limit on the directory name containing our conduit dll (mozABConduit.dll). If the path to the dll is longer than 110 characters, it fails to acknowledge the DLL as a valid conduit. This dll now lives in your user profile extension directory with this change because it's a real extension now. However that path alone on windows takes up almost 100 characters. That only leaves ~10 characters for your user name in the profile dir and the name of the extension (usually palmsync@mozilla.org). For now, I made the name of the extenion be t@t as that was the shortest string I could think of. But users with long profile names will clearly run into this issue.

	Scott MacGregor Reporter
	Comment 4 • 19 years ago

changes to the xpcom component palmsync dll to call PalmSyncInstall.exe the first time the app is run after installing the extension. When EM gives us an uninstall notification, we clear our conduit registration pref so we'll re-run PalmSyncInstall.exe the next time the extension is installed. 

Code clean up for some observer notification (we were adding ourselves as observers to the same topics multiple times) and category registration code (we were never unregistering).

	Scott MacGregor Reporter
	Comment 5 • 19 years ago

Here's an XPI built on the code in the patches above.

Please don't point the mozillazine thunderbird palm community at this. It needs more testing.

Just drag the XPI into the thunderbird extension manager. You should see it act like a regular extension which you can later uninstall.

	David :Bienvenu
	Comment 6 • 19 years ago

Comment on attachment 209032 [details] [diff] [review]
palmsync dll changes to support calling PalmSyncInstall.exe

cool, Scott.

	Scott MacGregor Reporter
	Comment 7 • 19 years ago

FYI, I let the extension name / guid be as small as possible: "p@m" (short for palmsync@mozilla.org) in order to minimize the number of users who won't be able to run this extension because the directory path for the extension files is too long for the palm desktop app to handle. I'm not sure there's anything we can do here other than pester Palm.

	Scott MacGregor Reporter
	Comment 8 • 19 years ago

Some of these changes are things you've already seen.

1) Call PalmSyncInstall.exe /us (silent uninstall) when we receive the uninstall notifcation from the extension manager. This causes us to unregister ourselves with hot sync when the extension is uninstalled.

2) PalmSyncInstall changes to stop looking for a local version of CondMgr and HSAPI.dll if we can't find the Palm Desktop version of those files. 

3) Stop putting the forementioned DLLS into the palm sync XPI.

4) Changed the wording of one of the install dialog strings based on feedback from Gordon.

	Scott MacGregor Reporter
	Comment 10 • 19 years ago

I've landed the build changes on the trunk and the thunderbird 2.0 branch so we can deal with further tweaks as incremental changes.

I'd still like to figure out a better solution to the long path name issue for palm desktop. And there might be some further install/uninstallation steps we might have to hook up.

Leaving the bug open for further tweaks and until the extension actually shows up on addons.mozilla.org.

	Ian Neal
	Comment 11 • 19 years ago

Latest checkin seems to have broken SeaMonkey 1.8 Branch builds (and will break trunk builds once PalmSync SDK is re-enabled)
http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla1.8-SeaMonkey/1138068120.6317.gz

	Scott MacGregor Reporter
	Comment 12 • 19 years ago

I wasn't aware anyone was building this. Sorry.

	Kevin Teuscher
	Comment 13 • 19 years ago

(In reply to comment #7)
> FYI, I let the extension name / guid be as small as possible: "p@m" (short for
> palmsync@mozilla.org) in order to minimize the number of users who won't be
> able to run this extension because the directory path for the extension files
> is too long for the palm desktop app to handle. I'm not sure there's anything
> we can do here other than pester Palm.
> 

Could you use the capability to use the registry in window to point at a directory other than the profile directory (e.g. the Palm Desktop directory) and install the extension in that location?

Kevin

	Wayne Mery (:wsmwk)
	Comment 14 • 19 years ago

install on a windows 98SE PC that never had TB, Palm Desktop and palmsync installed. result after TB restart:

"Failed to register the Thinderbird Palm Sync Support Proxy Dll while installing the Thunderbird Address Book Palm Scyn Conduit."

similar to bug 214407 comment 172 (he has since resolved his problem) so not a new problem obviously.

uninstalled and reinstalled a couple times with same results.

	Gordon Dickens
	Comment 15 • 19 years ago

Hey Scott,

IMHO, the 110 character pathname limitation may be a showstopper. Can we put the extention here instead?:

C:\Program Files\Mozilla Thunderbird\extensions\p@m\

As an example, it appears that palmsync-1.5.xpi will not install on one of the computers that I use to test your Palm Sync pre-release versions because of this limitation. 

Thanks,

Gordon

	Wayne Mery (:wsmwk)
	Comment 16 • 19 years ago

(In reply to comment #3)
> 
> 3) Palm desktop is old! They have a 110 character limit on the directory name
> containing our conduit dll (mozABConduit.dll). If the path to the dll is longer
> than 110 characters, it fails to acknowledge the DLL as a valid conduit. This
> dll now lives in your user profile extension directory with this change because
> it's a real extension now. However that path alone on windows takes up almost
> 100 characters. 

Scott - what is the field/structure name(s) and your reference?  

from Table A.1. at http://www.palmos.com/dev/support/docs/conduits/win/Intro_CfgEntries.html

"Conduit [Config. Entry]
Name of the conduit DLL file. If this entry is only a filename, the DLL must be in the HotSync Manager executable's directory *or in the current Windows user's PATH*. If it is a path and filename, you can put the DLL in any directory."

Is it possible to use PATH?


And from http://www.codeproject.com/ce/PalmOS_II.asp something which bears on the issue of versions:

"But wait! Palm has CDK 6.0.1. Isn't that newer and better?

Yes and no. CDK 6.0.1 provides minor updates on the generic conduit framework, integrates its help into the MSDN Library, supports system and user conduits, as well as has support for Palm OS 5.0 and up. The development tools are backwards compatible with Palm OS 3.5, except the generic conduit framework. Unfortunately for us, the framework was redesigned to target the extended database API, which Palm OS 3.5 does not support (only classic databases). This is likely the reason why Palm has kept the older 4.0.3 release online for download."

	Wayne Mery (:wsmwk)
	Comment 17 • 19 years ago

Scott, what code changed to eliminate need for contacts sidebar?

	Scott MacGregor Reporter
	Comment 18 • 19 years ago

(In reply to comment #13)
> (In reply to comment #7)
> > FYI, I let the extension name / guid be as small as possible: "p@m" (short for
> > palmsync@mozilla.org) in order to minimize the number of users who won't be
> > able to run this extension because the directory path for the extension files
> > is too long for the palm desktop app to handle. I'm not sure there's anything
> > we can do here other than pester Palm.
> > 
> 
> Could you use the capability to use the registry in window to point at a
> directory other than the profile directory (e.g. the Palm Desktop directory)
> and install the extension in that location?
> 
> Kevin
> 

I thought that capability refers to having fx/tb auto discover XPI files in other install locations by looking them up in the windows registry. I don't think that means you can actually control the directory it gets installed to, but maybe I'm wrong on that front.

	Kevin Teuscher
	Comment 19 • 19 years ago

(In reply to comment #18)
> I thought that capability refers to having fx/tb auto discover XPI files in
> other install locations by looking them up in the windows registry. I don't
> think that means you can actually control the directory it gets installed to,
> but maybe I'm wrong on that front.
> 
Actually it allows you to expand the XPI in a location and specify the location in the registry.  At the next start-up the program will recognize the extension and use it from the location noted in the registry as if it were installed in the profile extension location

For example,
1. Grab this zip file: http://mozilla.doslash.org/stuff/helloworld.zip
2. unzip it to: "c:\temp" (keeping the directory structure of the zip)
3. Create the key HKCU\Software\Mozilla\Firefox\Extensions
4. Create the value (REG_SZ): helloworld@mozilla.doslash.org
5. Set the value to: 'c:\temp\helloworld'
6. Start Firefox and see that you have the "Hello World" extension installed


I would think that we could use this and just install the palmsync extension in C:\Palm or c:\Program Files\PalmOne and be able to get around the path name length limitations.

Kevin

	Scott MacGregor Reporter
	Comment 20 • 19 years ago

That's an interesting idea Kevin. I'm not sure how an extension would do that though. For an extension to be up on addons.mozilla.org, it needs to be an install.rdf based extension. Your install.rdf would have to install a dummy extension perhaps which installed some JS the next time the product was launched which in turn installed the real palm sync extension and added the registry keys. Then you'd have to restart again to get the palm sync extension installed. I think in that scenario you'd also end up with two extensions in the extension manager: one for your bootstrap extension and the other one for the real palm sync extension. 

Not ruling this idea out, I'm just not sure how it would fit together. 

	Kevin Teuscher
	Comment 21 • 19 years ago

Another approach vector that could be taken is to install the sync conduit and as part of the install, add the registry entries and extension file.  That way you could distribute an executable to do all of this.  You already have to run an executable after installing the extension in order to install the conduit, so why not just reverse the install vector.

The drawback is that you can't distribute from amo, but maybe you could have the install link go to the TB product page where you have an "extras" section in the downloads.

Kevin

	Wayne Mery (:wsmwk)
	Comment 22 • 18 years ago

Scott, did you do any work with both VC8 and CDK 4.0.3?

	Wayne Mery (:wsmwk)
	Comment 24 • 18 years ago

At least one tester got burned by the 110 character limit, demonstrating a release note is needed.  The restriction is profile-name + windows-user-name <= 20 characters.  

Can palmsync's install process test if it's attempting to install into a directory name that is too long?

On the issue of workarounds I tested global install with 
  thunderbird -install-global-extension palmsync.xpi 
to put the extension into the program directory instead of profiles, noted at
http://kb.mozillazine.org/Installing_extensions#Global_installation
It works but the down side is you can't use the addons UI to install.

In investigated a dead end - hoping it might be possible to install into /documents and settings/<user>/application data/Thunderbird/Extensions but Mossop tells me that support for this dir was removed (for firefox anyways) Feb 2006


Scott, David, Vista needs to be tested (I forgot to mention this a few weeks ago).  Bug 374151 mentions a crash but details are lacking.  I'll try to get more info into the bug, but as of now I don't have Vista and don't have anyone to test it.

	Gordon Dickens
	Comment 25 • 18 years ago

(In reply to comment #24)
> At least one tester got burned by the 110 character limit, demonstrating a
> release note is needed.  The restriction is profile-name + windows-user-name <=
> 20 characters.
>
> Can palmsync's install process test if it's attempting to 
> install into a directory name that is too long?

Beware that modifying the profile name in the user profile interface is not adequate since that does not actually change the profile folder name at: C:\Documents and Settings\<user name>\Application Data\Thunderbird\Profiles\<profile folder name>.  The user will need to create a new profile that meets the 110 character limitation and then manually copy their files from their old profile to a the new profile.  I believe that a procedure should be written for creating a profile that meets the 110 character limit and for the migration of data to the new profile.  I agree with Wayne that we also need to have PalmSync's install process detect if the 110 character limit is not being met and then report back to the user accordingly.

	Mark Banner (:standard8)
	Comment 26 • 18 years ago

(In reply to comment #24)
> At least one tester got burned by the 110 character limit, demonstrating a
> release note is needed.  The restriction is profile-name + windows-user-name <=
> 20 characters.  

I don't know which bit of code is used to set the directory name, but is it possible we could use the short folder name/path for palm sync? nsILocalFileWin supports this: http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsILocalFileWin.idl

	timeless
	Comment 27 • 18 years ago

there's no guarantee that short paths exist. NT isn't required to offer them. (seriously, it's entirely optional and a system can easily have the feature disabled)

	Wayne Mery (:wsmwk)
	Comment 28 • 18 years ago

(In reply to comment #27)
> there's no guarantee that short paths exist. NT isn't required to offer them.

but by default systems come with it turned on, no?

does this describe the mechanism you are referring to?  http://support.microsoft.com/kb/121007/


Does vista use the exact same directory name - documents and settings?

	Wayne Mery (:wsmwk)
	Comment 29 • 18 years ago

I tested a short path name of C:\DOCUME~1\<username>\APPLIC~1\... and it works fine (not surprising).  It gives us almost 20 additional characters and might be a way out of this predicament.  

I suspect the current limitation 20 characters is unrealistic and going to get us burned with someone or some company that has standards where the user name easily exceed 15 characters.

The question is whose executable code sets the path name, ours or palm's?

FWIW, here is a palm registry entry that shows an 8.3 name but perhaps that we set by windows and not palm.

	Wayne Mery (:wsmwk)
	Comment 30 • 18 years ago

Scott, what's your opinion on using a short path name?  Are there issues that still need to be addressed?

The choices so far are
* find another install location (like program directory)
* document a restriction on length of username+profilename 
* 8.3 pathname

I googled and found very few references that talk about it let alone recommend disabling it (a handfull) - all in the context of using products that create hundreds to thousands of files in a directory.  I think we are more likely to encounter people with user names long enough to fail installation than people who have disabled 8.3.

	David :Bienvenu
	Comment 31 • 18 years ago

I wonder if the Palm stuff itself works without 8.3 file names - it's possible it doesn't, in which case this would be a no brainer...

	Worcester12345
	Comment 32 • 18 years ago

(In reply to comment #31)
> I wonder if the Palm stuff itself works without 8.3 file names - it's possible
> it doesn't, in which case this would be a no brainer...
> 
Has anyone bothered asking Palm any of this? I guess it wouldn't hurt for a developer to pick up the phone. It would be mutually beneficial in fact.

	Wayne Mery (:wsmwk)
	Comment 33 • 18 years ago

max windows XP username length is 20 characters
mozilla profile name appears to have no imposed max length (I tested ~70 char)

possible solution incorporating 8.3:
get short path
if failure, get normal path
if path length > max, issue error and exit
[register conduit]

David, can you do the patch? 

	Scott MacGregor Reporter
	Comment 34 • 18 years ago

if forcing 8.3 paths fixes this problem, that sounds good to me if it doesn't work that way already!

	Wayne Mery (:wsmwk)
	Comment 35 • 18 years ago

apparently names in vista are c:\username\... with symbolic names (junctions) that map to "Documents and Settings"\username  to help software that still expects files to be at the "old" locations.  Reports differ however about how this is implemented, for example http://www.devsource.com/article2/0,1895,1999637,00.asp and http://4sysops.com/archives/documents-and-settings-in-vista/ 

I did not find information about whether 8.3 name capability changed in vista.

	Mark Banner (:standard8)
	Comment 36 • 18 years ago

(In reply to comment #34)
> if forcing 8.3 paths fixes this problem, that sounds good to me if it doesn't
> work that way already!

Note that canonicalPath won't force 8.3 paths, but it will use them where possible.

	Wayne Mery (:wsmwk)
	Comment 37 • 18 years ago

Scott in comment #34:
> if forcing 8.3 paths fixes this problem, that sounds good to me if it doesn't
> work that way already!

Path is determined here, set to where PalmSyncInstall.exe is running from
http://lxr.mozilla.org/seamonkey/source/xpinstall/wizard/windows/palmsync/PalmSyncInstall.cpp#148

I got positive results today with long path name. I tested trunk suiterunner xpi Mark gave me, installed into a path with 192 characters, with seamonkey 1.1.1 and it ran fine.  palm HSM is version 4.1  

We don't have documented under what conditions the sync with >110 path failed.  Does anyone remember whether it's HSM can't call palmsync.dll, or palmsync.dll fails after being started?  Or have an older version of HSM/palm desktop that can be tested?

	Wayne Mery (:wsmwk)
	Comment 38 • 18 years ago

Mark, the last xpi for bug 378723 worked but I forgot to mention it throws up a small dialog box 
   Test [window title]
   results: ##"
where ## appears to be a return code from somewhere for both successful and unsuccessful installs. never seen it before and I don't see where it comes from in the source. Does not happen during uninstall. 

David, in addition to fixing the issue above, what should happen before the 8.3 path portion of the Mark's patch can be checked in to branch?  

David/Scott - is dist\xpi-stage mentioned in comment 3 a publicly accessible directory? 

	Mark Banner (:standard8)
	Comment 39 • 18 years ago

(In reply to comment #38)
> Mark, the last xpi for bug 378723 worked but I forgot to mention it throws up a
> small dialog box 
>    Test [window title]
>    results: ##"
> where ## appears to be a return code from somewhere for both successful and
> unsuccessful installs. never seen it before and I don't see where it comes from
> in the source. Does not happen during uninstall. 

Can you try the latest suiterunner build version (ftp://ftp.mozilla.org/pub/mozilla.org/seamonkey/tinderbox-builds/tpol-trunk/)? I think this is just some debug code I've got hanging around in my build. If it works (i.e. not the 50% problem) then its probably the right version - I think ftp.mozilla.org has the latest, if not wait a while and try again ;-)

You'll still need to change the directory name.

	Mark Banner (:standard8)
	Comment 40 • 18 years ago

(In reply to comment #38)
> David/Scott - is dist\xpi-stage mentioned in comment 3 a publicly accessible
> directory? 

dist\xpi-stage is a directory created as part of a build. Its not on a ftp server if thats what you were thinking.

	Mark Banner (:standard8)
	Comment 41 • 18 years ago

This path changes the INSTALL_EXTENSION_ID variable to match that in the RDF file. This means when we build it in tree (developer or suiterunner) it'll go into extensions/ in the same directory as the id in the rdf file so it'll register properly as an extension when the app runs up.

Note that I changed install.rdf to use INSTALL_EXTENSION_ID in case we ever decide to change it in future...

	Mark Banner (:standard8)
	Comment 42 • 18 years ago

(In reply to comment #41)
> Created an attachment (id=263381) [details]
> Change installation path to match extension id.

I've just checked this patch in.

	Wayne Mery (:wsmwk)
	Comment 43 • 18 years ago

David, Scott, should all of attachment 209150 [details] [diff] [review] and attachment 209032 [details] [diff] [review] be on branch?

	Wayne Mery (:wsmwk)
	Comment 44 • 18 years ago

adapted for branch from mark's trunk patches in bug 378723 - attachment 263024 [details] [diff] [review] and attachment 263211 [details] [diff] [review]

Mark, does it look like I got the right pieces?

David, I don't have branch source or build so I don't have the means to verify correctness of source nor ability to compile

	Mark Banner (:standard8)
	Comment 45 • 18 years ago

(In reply to comment #44)
> Created an attachment (id=266394) [details]

> Mark, does it look like I got the right pieces?

Well it looks a little strange as I think you've knitted them together, but it does look like you've got the right bits.

	Wayne Mery (:wsmwk)
	Comment 46 • 18 years ago

David, will also need checkin branch and trunk

	Wayne Mery (:wsmwk)
	Comment 47 • 18 years ago

Comment on attachment 266927 [details] [diff] [review]
point to palmsync kb article

patch flag for install.rdf mozilla\mailnews\extensions\palmsync

	David :Bienvenu
	Comment 48 • 18 years ago

this patch is mal-formed - you can't just add the two diffs together; you need to merge the lines that changed into one diff. I'll try to figure out what specifically needs to change, though I'll be flying blind a bit.

	Wayne Mery (:wsmwk)
	Comment 49 • 18 years ago

looks better. hope it applies

	David :Bienvenu
	Comment 50 • 18 years ago

Comment on attachment 266946 [details] [diff] [review]
8.3 patch v2 for branch (adapted from bug 378723)

applies and builds, thanks! I'll try package up an xpi for you asap.

	Wayne Mery (:wsmwk)
	Comment 51 • 18 years ago

(In reply to comment #47)
> (From update of attachment 266927 [details] [diff] [review])
> patch flag for install.rdf mozilla\mailnews\extensions\palmsync

David, attachment 266927 [details] [diff] [review] is for trunk.
For branch, I don't know where the file is - don't see it in mozilla\mailnews\extensions\palmsync nor in xpinstall/wizard/windows/palmsync/.

	Wayne Mery (:wsmwk)
	Comment 52 • 18 years ago

Scott in comment #3:
> Created an attachment (id=208814) [details]
> make palm sync an install.rdf based extension 
> 
> Changes to make us build an install.rdf based extension for palm sync, and to
> automate the building of the extension.
> 
> 1) The build system automatically generates a palmsync-*version*.xpi file in
> dist\xpi-stage now. That file can be hosted on addons.mozilla.org once we fix
> the rest of the issues listed below.

Mark, David, 

presently the extension version is keyed to gecko, ala 1.8.x or 1.9.x. Would it not make sense for it to be keyed to the version of thunderbird or seamonkey by  changing the last line of 
 XULAPP_DEFINES = \
         -DINSTALL_EXTENSION_ID=$(INSTALL_EXTENSION_ID) \
         -DTHUNDERBIRD_VERSION=$(THUNDERBIRD_VERSION) \
         -DSEAMONKEY_VERSION=$(SEAMONKEY_VERSION) \
         -DEXTENSION_VERSION=$(MOZILLA_VERSION) \

to 
ifdef MOZ_XUL_APP
         -DEXTENSION_VERSION=$(SEAMONKEY_VERSION) \
else
         -DEXTENSION_VERSION=$(THUNDERBIRD_VERSION) \

Or, a better alternative?

	Mark Banner (:standard8)
	Comment 53 • 18 years ago

(In reply to comment #52)
> presently the extension version is keyed to gecko, ala 1.8.x or 1.9.x. Would it
> not make sense for it to be keyed to the version of thunderbird or seamonkey by
>  changing the last line of 
...
> Or, a better alternative?

Given that the extension dependencies are such that it appears to need to be built for each specific version of tree, I think that linking to the gecko version isn't too bad (and we certainly do that with some of the other in-tree extensions).  However if you did change it, then I'd suggest just sticking in a constant that we can increment when we think it appropriate.

	Wayne Mery (:wsmwk)
	Comment 54 • 18 years ago

(In reply to comment #53)
>> Given that the extension dependencies are such that it appears to need to be
> built for each specific version of tree, I think that linking to the gecko
> version isn't too bad (and we certainly do that with some of the other in-tree
> extensions).  

what example(s) should I look at?  

I should have elaborated on my concerns that prompted the question: 
a) version was not previously visible in the extension manager, so no one has seen it before and we can do whatever we want.  
b) almost everyone who sees the new xpi now asks me "why is the version 1.8.x?" (including Dave) 
c) min+max version checks are tied to the application version, not the tree version.
d) the build process names the xpi with the application version


> However if you did change it, then I'd suggest just sticking in a
> constant that we can increment when we think it appropriate.

Is this relevant? ... Scott comments in the makefile
 # We should really pull THUNDERBIRD_VERSION from
 # mail/config/version.txt but we can't be assured
 # that we've even pulled that file. So we hardcode them.

I have no idea why.

	Robert Kaiser
	Comment 55 • 18 years ago

That comment is bogus nowadays.

Anyways, I think using the Gecko version is really correct. Version 1.0 of the PalmSync extension was shipped with Mozilla 1.0, even though it may not have been publicly visible. And versioning the same extension differently in SeaMonkey and Thunderbird and maybe Penelope is, forgive me, bullshit. DOMI is also using the Gecko version, and nobody complains about that there.

	Wayne Mery (:wsmwk)
	Comment 56 • 18 years ago

(In reply to comment #55)
> That comment is bogus nowadays.

you mean about mail/config/version.txt ?


> Anyways, I think using the Gecko version is really correct. Version 1.0 of the
> PalmSync extension was shipped with Mozilla 1.0, even though it may not have
> been publicly visible. And versioning the same extension differently in
> SeaMonkey and Thunderbird and maybe Penelope is, forgive me, bullshit. DOMI is
> also using the Gecko version, and nobody complains about that there.

Can the extension installation process check max/min version against gecko?
If not, then I submit versioning based on gecko doesn't make sense from that viewpoint alone.

Gecko version may be correct from tech viewepoint, but it has it's problems - at least for Thunderbird. And unlike suite/seamonkey  the distribution location at http://ftp.mozilla.org/pub/mozilla.org/thunderbird/extensions/palmsync/ has clearly linked the extension to a specific version of thunderbird. 

But your comment goes to another issue which has not been discussed - is it possible to have one extension for both seamonkey (or suiterunner) and thunderbird. i.e. should there be a convergence and just one version of palmsync for both suite and thunderbird? 

	Robert Kaiser
	Comment 57 • 18 years ago

(In reply to comment #56)
> (In reply to comment #55)
> > That comment is bogus nowadays.
> 
> you mean about mail/config/version.txt ?

Did you see it actually hardcoded there? I don't think so, it uses THUNDERBIRD_VERSION and this var is set by configure, which directly sets it from mail/config/version.txt and client.mk ensures we pull it.
So, actually, about everything the comment is saying is wrong nowadays.


> Can the extension installation process check max/min version against gecko?

That doesn't matter (but actually, work is going on to support that). This is the version of the palmsync extension, not of whatever app it is installed in.

> should there be a convergence and just one version of
> palmsync for both suite and thunderbird? 

I didn't know there even might be different versions for the two apps. We're both using exactly the same code, right?

Oh, and the distribution  location is wrong anyways if it is on ftp.m.o as nothing that is only on ftp.m.o should be official releases, from what I know, as releases.m.o or download.m.o have much better load balancing and mirroring. Anyways, an extension is an add-on and belongs on AMO. And there, it can quite easily be marked to support both Thunderbird and SeaMonkey and show up for both, btw.

	Mark Banner (:standard8)
	Comment 58 • 18 years ago

(In reply to comment #57)
> (In reply to comment #56)
> > should there be a convergence and just one version of
> > palmsync for both suite and thunderbird? 
> I didn't know there even might be different versions for the two apps. We're
> both using exactly the same code, right?

The extension did have a time of being slightly different code, but now we're both XUL apps, the code base is common again (I specifically made it to be).

My only concern here currently is that the palm sync code is at the moment being linked directly against the mailnews shared libraries because it is using class implementations from within mailnews/addrbook. Therefore if you compile it against Thunderbird for instance, I'm not sure how reliable/sensible it would be to use against SeaMonkey because essentially it'll be a slightly different shared library that its calling up. Possibly its not too much of an issue if we're using exactly the same code base (which is what we do on stable branches) and tie it to specific versions of the code.

IMHO this is really something we should fix, though it will possibly make the palm sync extension bigger/more complex. Fixing this would mean that we'd only need to update the extension when particular AB interfaces changed.


So as for versioning, I think the version should be independent of which app we're building it against. At the moment I don't think there is anything wrong with tying it to the gecko version, as you generally need a specific code base to run it against. We could if we want use specific/manually incrementing version numbers, but I suggest that if we did then we should definitely fix the above issue with the linking.

Also remember that the version number is independent of the app version that it gets installed into - the app version is restricted independently by install.rdf. We don't have to limit it to one specific version of the app, just at the moment its better to. 

As for shipping I think SeaMonkey is happy to continue shipping with the installer. Though, publishing the extension to AMO would be a good idea (again once we fix the lib issues) - Thunderbird users will be able to install and download it, SeaMonkey users would be able to pick up updates if we did some feature improvements.

	Wayne Mery (:wsmwk)
	Comment 59 • 18 years ago

(In reply to comment #55)
> DOMI is also using the Gecko version, and nobody complains about that there.

the major difference is DOMI is a technical tool and anyone who uses DOMI clearly understands it operates on gekco related elements. But what average user would expect an extension like palmsync, which is address book oriented, to have no dependency on the "application" and be strictly dependent to back-end code? 


For my own education, what other classic extensions are dependent only on gecko?

(In reply to comment #57)
>Did you see it actually hardcoded there? I don't think so, it uses
>...

I know little of the history of palmsync and nothing of the build process nor how it operates, so thanks for the info.


>> Can the extension installation process check max/min version against gecko?
>
>That doesn't matter (but actually, work is going on to support that). This is
>the version of the palmsync extension, not of whatever app it is installed in.

it matters to me if we move toward a common extension.


(In reply to comment #58)
>However if you did change it, then I'd suggest just sticking in a
>constant that we can increment when we think it appropriate.
(In reply to comment #58)
>We could if we want use specific/manually incrementing version numbers, but I 
>suggest that if we did then we should definitely fix the above issue with the
>linking.

A palmsync version that is independent of both gecko and the application may be a good way to go.  Users are accustomed to this with most other extensions. 

What is needed for the linking issue?  An API?

OTH, if http://wiki.mozilla.org/Mozilla2:Device_Sync is delivered within 1-2 years it might not be worth the effort.

	Robert Kaiser
	Comment 60 • 18 years ago

Wayne, your arguments contradict each other:
First, so say that we should use the same version as the app, as the user expects that. A few lines later, you say a version independent of both gecko and the app is what users are accustomed to.

So why can the version independent of the app just happen to be the version that "accidentally" is also used by Gecko?

IMHO, that's as good as a completely independent version but has the good side that no developer needs to explicitely care about bumping it for new versions, as that happens automatically.

	Wayne Mery (:wsmwk)
	Comment 61 • 18 years ago

you're right I may have contradicted myself :)

So there needs to be two packages - one for Thunderbird and one for Suite. And there is a difference of opinion regarding version numbers. 

New info regarding version numbering - sdwilsh and others on #addons state AMO "updates are done based on version number alone".  So if palmsync is tied to gecko version then we can't push a palmsync update, be it fixes or enhancements, without someone else changing the gecko version. Not a good dependency in my opinion.  So versioning based on gecko I think need to be reconsidered in this new light.

More details about versioning and updating at http://developer.mozilla.org/en/docs/Extension_Versioning%2C_Update_and_Compatibility#Automatic_Extension_Update_Checking

I wonder too, since hotsync can occur without thunderbird or suite knowing about it, do we need to worry whether thunderbird might update while hotsync is in progress and the conduit is in active use?

	Wayne Mery (:wsmwk)
	Comment 62 • 18 years ago

(In reply to comment #61)
> I wonder too, since hotsync can occur without thunderbird or suite knowing
> about it, do we need to worry whether thunderbird might update while hotsync is
> in progress and the conduit is in active use?

I'll answer my own dumb question - because an extension update requires a restart probably not - that is, if a user is being smart s/he won't intentionally restart thunderbird while a hotsync is in progress.

	Robert Kaiser
	Comment 63 • 18 years ago

(In reply to comment #61)
> So there needs to be two packages - one for Thunderbird and one for Suite.

We don't.

> New info regarding version numbering

No new info for people like me and Mark, but maybe for you ;-)
Anyways, this even more means that we can't go with the SeaMonkey/Thunderbird version numbers but need to do a separate one. Still, we could spare that and go with the Gecko number if we agree that changes happen rarely enough that this is sufficient.

	David :Bienvenu
	Comment 64 • 18 years ago

Comment on attachment 266927 [details] [diff] [review]
point to palmsync kb article

landed on trunk and 1.8.1 branch

	Wayne Mery (:wsmwk)
	Comment 65 • 18 years ago

"keep on top" needs to be added to the palmsync install dialog yes/no prompt so that thunderbird's/seamonkey's restart doesn't cover it. so need to add MB_TOPMOST ~line 201 of palmsyncinstall.cpp 
     if (MessageBox(NULL, msgStr, appTitle, MB_YESNO) == IDYES) 

	Wayne Mery (:wsmwk)
	Comment 66 • 17 years ago

follow up to comment 65.  Please checkin to branch and trunk after approval.

  if (MessageBox(NULL, msgStr, appTitle, MB_YESNO | MB_TOPMOST) == IDYES)
to keep palmsyncinstall.exe dialog box on top so it is visible to user after thunderbird restarts following extension installation via addons.

David provided an extension back in June (6/22 iirc) with this modification and it's gotten lots of testing from many people but was never submitted as a patch for checkin.

AFAIK, this is the last patch needed before making the extension public, except perhaps versioning.

	Wayne Mery (:wsmwk)
	Comment 67 • 17 years ago

Comment on attachment 291942 [details] [diff] [review]
Add MB_TOPMOST to MessageBox

(it didn't keep my review assignment)

	Worcester12345
	Comment 68 • 17 years ago

Missed the Target Milestone: 	 Thunderbird2.0 , but there is no way to change this field.

	Wayne Mery (:wsmwk)
	Comment 69 • 17 years ago

Comment on attachment 291942 [details] [diff] [review]
Add MB_TOPMOST to MessageBox

revised patch file coming up

	Wayne Mery (:wsmwk)
	Comment 70 • 17 years ago

Previous patch attachment 291942 [details] [diff] [review] never made it to branch or trunk afaik.  We will want checkin of this patch (NPOTB) to both before building extension from those sources for the purpose of pushing Thunderbird extension to AMO. See comment 65 and 66

David, regarding review - the original line 
  if (MessageBox(NULL, msgStr, appTitle, MB_YESNO) == IDYES)
appears twice in the program. This patch touches only the install code. uninstall code is left untouched because afaict MB_TOPMOST is not required for proper uninstall behavior.  However, consider in review whether we should be symmetric, i.e. that uninstall should also be changed.  


Also, some incomplete business about Vista - it never got good testing - I'm awaiting feedback about what works on Vista (ref: bug 422494 - unclear if problem is palm's vendor or palmsync's)

	David :Bienvenu
	Comment 71 • 17 years ago

adding checkin-needed, both branch and trunk, per Wayne's prev comment.

	Mark Banner (:standard8)
	Comment 72 • 17 years ago

(In reply to comment #70)
> Created an attachment (id=320988) [details]
> Add MB_TOPMOST to MessageBox to prevent install dialog getting hidden

Checking in mailnews/extensions/palmsync/installer/PalmSyncInstall.cpp;
/cvsroot/mozilla/mailnews/extensions/palmsync/installer/PalmSyncInstall.cpp,v  <--  PalmSyncInstall.cpp
new revision: 1.14; previous revision: 1.13

David, can we check this in on branch as NPOTB? If so, I'll do it tomorrow.

	David :Bienvenu
	Comment 73 • 17 years ago

yes, that's fine, as long as you make that clear in the checkin comment - afaik, they're not doing any tagging or building on the branch.

	Mark Banner (:standard8)
	Comment 74 • 17 years ago

(In reply to comment #72)
> (In reply to comment #70)
> > Created an attachment (id=320988) [details] [details]
> > Add MB_TOPMOST to MessageBox to prevent install dialog getting hidden

Checked in on branch:

Checking in PalmSyncInstall.cpp;
/cvsroot/mozilla/xpinstall/wizard/windows/palmsync/Attic/PalmSyncInstall.cpp,v  <--  PalmSyncInstall.cpp
new revision: 1.9.18.2; previous revision: 1.9.18.1
done

with a=KaiRo over irc.

	Mark Banner (:standard8)
	Comment 75 • 17 years ago

Is there anything else we need to do on this bug now or can it be closed? AFAIK the build mechanism works, and other issues should be covered by other bugs.

	Wayne Mery (:wsmwk)
	Comment 76 • 17 years ago

Scott and David have longer history here and can better comment about closing, but from my perspective the important open issues are covered in other bugs.  

If the thinking is to push the extension to AMO after this is closed, one relnote is appropriate about bug 422494 / problems with HSM 7, palm desktop 6.

	Mark Banner (:standard8)
	Comment 77 • 17 years ago

(In reply to comment #76)
> Scott and David have longer history here and can better comment about closing,
> but from my perspective the important open issues are covered in other bugs.  
> 
> If the thinking is to push the extension to AMO after this is closed, one
> relnote is appropriate about bug 422494 / problems with HSM 7, palm desktop 6.
> 
I believe pushing the extension to AMO is going to be a manual process (although the xpi could be done dynamically). Therefore I think we can close this bug now. Its already done a lot.