245397 - OK in conflict dialog doesn't work

Status: RESOLVED INCOMPLETE

(Core Graveyard :: Profile: Roaming, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040601 MultiZilla/1.6.3.2f
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040601 MultiZilla/1.6.3.2f

It appeared to me on Windows and Solaris with current nightlies that the Roaming
files conflict dialog does not go away when I select which settings to take
(remote vs. local) and press OK - the button simply does nothing. This happens
both with https and ftp methods and different roaming accounts on the server.

I guess - but am not sure - that this only appears if there are more than seven
files that conflict.

Reproducible: Sometimes
Steps to Reproduce:
1. Set up roaming
2. Make it that several files are different on server and client so that
conflicts appear
3. In the Roaming files conflict dialog, select which files to keep, e.g. all
from locally
4. Try to press the OK button
Actual Results:  
In some cases, the OK button does nothing at all. Cancel is your only way out.

Expected Results:  
The OK button should accept the user's choice and initiate whichever process is
to handle it.

Comment 1

[Ben Bucksch (:BenB)]

jhatlak at <http://forums.mozillazine.org/viewtopic.php?p=567864#567864> says he
sees this, too. CONFIRMing.

Are you sure that all the files you selected exist? There are some problems
around roaming files which don't exist...

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #1)
> jhatlak at <http://forums.mozillazine.org/viewtopic.php?p=567864#567864> says he
> sees this, too. CONFIRMing.

This is actually me, the reporter. ;-)

> Are you sure that all the files you selected exist? There are some problems
> around roaming files which don't exist...

I do know that, but yes, all the files selected for roaming exist both on the
server and in the profile directory. I really think this is a problem of the
number of entries in the dialog. I had nine entries and couldn't click OK. Then
I deselected Bookmarks and Message views in Preferences and restarted Mozilla.
This time, I was able to press OK and syncing started.

Comment 3

[Ben Bucksch (:BenB)]

> Message views

I'd suspect that this (mailViews.dat) is the problem.

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #3)
> > Message views
> 
> I'd suspect that this (mailViews.dat) is the problem.

I don't think so - the OK button works fine if I only select Message views in
Preferences and trigger the conflict dialog...

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Just wanted to inform you that I now built from CVS again after checkin of bug
246710 and still see this problem. So it's probably not related to non-existing
files. And still, I see no error messages or whatsoever. :-(

Comment 6

[Ben Bucksch (:BenB)]

Please test against official nightly builds

Comment 7

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #6)
> Please test against official nightly builds

I did. No difference. Didn't you receive my e-mail?

So, since this bug is really annoying: Has anyone else ever got more than 7
items in the Roaming files conflict dialog at all? Otherwise you won't see this
bug! It is 100% reproducible for me and appears with ALL builds, be they
official or not.

Is there anything I can do to get better debug information? Anyone a patch
giving some console dump?

Comment 8

[Henrik Gemal]

The reason for the Ok button doesnt work:

Error: uncaught exception: [Exception... "Component returned failure code:
0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIDialogParamBlock.SetInt]"  nsresult:
"0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
chrome://sroaming/content/transfer/conflictResolve.js :: onOK :: line 179" 
data: no]

using build 20040628

Comment 9

[Ben Bucksch (:BenB)]

Ah, thanks, that's useful. (or if you want to take the bug...)

Comment 10

[Pete Zha]

That's probably because the max number of parameter list is not enough

Comment 11

[Pete Zha]

Attachment: patch

try this patch to see if it will resolve this problem

Comment 12

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #11)
> Created an attachment (id=152488)
> patch
> 
> try this patch to see if it will resolve this problem

No, unfortunately it does not.

But if I understand nsIDialogParamBlock.h correctly, SetNumberStrings does only
affect the number of /strings/ allowed to be passed. However, Henrik reported
something about an NS_ERROR_ILLEGAL_VALUE in /SetInt/. Maybe the result of
files->Count() is the problem? - just taking a guess here, I don't really know.

Comment 13

[Pete Zha]

(In reply to comment #12)
> (In reply to comment #11)
> > Created an attachment (id=152488)
> > patch
> > 
> > try this patch to see if it will resolve this problem
> 
> No, unfortunately it does not.
> 
> But if I understand nsIDialogParamBlock.h correctly, SetNumberStrings does only
> affect the number of /strings/ allowed to be passed. However, Henrik reported
> something about an NS_ERROR_ILLEGAL_VALUE in /SetInt/. Maybe the result of
> files->Count() is the problem? - just taking a guess here, I don't really know.

If you select all the item for roaming, this patch will take effect. Yes, you
are right, it's because count of file is too large. The max number of int value
set in nsDialogParamBlock is 8. and we can not reset it after create instance...
http://lxr.mozilla.org/seamonkey/source/embedding/components/windowwatcher/src/nsDialogParamBlock.h#62

Comment 14

[Pete Zha]

Attachment: patch to add more int in DialogParamBlock.

Comment 15

[Pete Zha]

Comment on attachment 162038 [details] [diff] [review]
patch to add more int in DialogParamBlock.

Ben, please review

Comment 16

[Pete Zha]

If above patch is not acceptable, we should probably use another way to pass the
file conflict info

Comment 17

[Ben Bucksch (:BenB)]

Comment on attachment 162038 [details] [diff] [review]
patch to add more int in DialogParamBlock.

I'm not sure, if I'm allowed to review this. The author danm is gone, I think.
It seems reasonable enough - why more default strings than default ints?

The clean fix would be to create a SetNumberStrings function like the current
SetIntStrings, esp. since we may well have more files than 16, so this patch
won't even fix the problem in all our cases.

Comment 18

[Ben Bucksch (:BenB)]

> create a SetNumberStrings function like the current SetIntStrings

eh, the other way around.

Comment 19

[Ben Bucksch (:BenB)]

Attachment: Fix, Part 1, v3 - Crashing

I implemented the above suggestion, but I am crashing in the dialogparam dtor.
Even if I comment out any or all of the delete[]s. No idea why. Help, please.

Comment 20

[Benjamin Smedberg]

This bug is filed in a bugzilla component related to pre-Firefox code which no longer exists. I believe it is no longer relevant and I am therefore closing it INCOMPLETE.

If you believe that this bug is still valid and needs to be fixed, please reopen it and move it to the Toolkit:Startup and Profile System product/component.