109997 - <wizard/> doesn't display (default or manual) radio selection on first launch

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[racham]

When the first page contains a set of choices represented as radiogroup, on the
first launch, <wizard/> is failing to display the selection made either
programatically via javascript or manually via user clicks. 

Here is something I noticed. Even though none of the radio buttons are selected,
I can proceed clicking next, it picks up the default (ex: mail account setup).
Also, I can manullay click on other radio choice and proceed next and the
selection is honored (ex: news account setup). Paint problem ?

If I close the wizard and relaunch, then everything works fine.

Comment 1

[racham]

Checked with today's build. This bug still exists.

Comment 2

[Peter Trudelle]

p2/nsbeta1+

Comment 3

[racham]

*** Bug 123884 has been marked as a duplicate of this bug. ***

Comment 4

[racham]

Data from bug 123884 which detailed steps to reproduce :

--begin

Trunk build 2002-02-06-03: WinMe
Trunk build 2002-02-06-09: Linux RH 7.1
Trunk build 2002-02-06-08: Mac 10.1

Overview: Open the Account Wizard and no radio button appears selected.

Steps to reproduce:
1. Select Edit|Mail & Newsgroups Account Settings
2. Select the Add Account button

Actual Results: 
- No radio button is auto selected
Workaround: Cancel out of the Account Wizard, reopen the Account Wizard and now
the Email Account radio button is selected.

Expected Results: The Email Account radio button should be selected by default
when the Account Wizard opens.



------- Additional Comment #1 From Ninoschka Baca 2002-02-06 11:06 -------

Marking nsbeta1 because this is not a good first experience

Additional Information:

Actual Results: 
- Acct Wiz 1st dialog: Progressing through the Account Wizard by selecting Next
and Back doesn't effect the Email Account radio button, it's still does not
appear selected with the black dot in the radio button. Only workaround is to
exit Account Wiz and open it again.

- Acct Wiz 2nd dialog, Server dialog: Interesting that the Server dialog's radio
buttons are performing as expected.


--end

Comment 5

[R.K.Aa.]

*** Bug 124572 has been marked as a duplicate of this bug. ***

Comment 6

[R.K.Aa.]

*** Bug 125222 has been marked as a duplicate of this bug. ***

Comment 7

[racham]

*** Bug 125262 has been marked as a duplicate of this bug. ***

Comment 8

[R.K.Aa.]

*** Bug 126696 has been marked as a duplicate of this bug. ***

Comment 9

[Blake Ross]

Attachment: something of a workaround

Comment 10

[Joe Hewitt (gone)]

reassigning to racham since the fix is all in account wizard

Comment 11

[racham]

Will try the workaround. Accepting...

Comment 12

[Donnie Cranford]

I cannot beleive this bug has been futured to moz 1.0, 
Who in Gods name would use a mail client if we cant even get the radio buttons
to paint correctly? This is stuff we need to pay attention to. I know if im a
normal user and go to setup mail/news and the button doesnt get painted im just
going to assume that the client is unusable. This needs to be brought back to
0.9.9 so that we can stop getting bad reviews because we let stuff like this
fall through the cracks. 

-dcran-

Comment 13

[R.K.Aa.]

*** Bug 128637 has been marked as a duplicate of this bug. ***

Comment 14

[Carsten Menke]

BTW, it worked for me with yesterdays branch build (March 3rd 2002), but it
still does not with the trunk build.

Comment 15

[Donnie Cranford]

Carsten,

Was this a new profile or a previous profile? I have only tested it with a new
profile, which if anyone installs mozilla for the first time, they WILL see this
bug. Which is why we need to move this to 0.9.9 rather than 1.0 nsbeta1+.

Comment 16

[Carsten Menke]

AFAIK it was a new Profile, because, I never completed to set up the profile to
see if this problem is solved. So everytime when I go Tasks->Adressbook for
example the wizard pops up. I hit always cancel, and with the trunk build the
radio button was *NOT* there, but with the branch build the radio button appeared.

Will build again tomorrow, can tell you if it's still the same then.

Comment 17

[racham]

Attachment: snapshot1 - problem with the above patch

Patch in there allows the first radio button to have 'selected' look. But the
problem comes when user clicks on the next/other radio item (please see the
snapshot). Along the rectangular box, we also need to change 'dot' selection to
not to confuse users. Radiogroup bindings/stlyes seem to be broken when we use
selected attribute on one of it's elements.

Comment 18

[Carsten Menke]

Donnie I think Racham got it exactly as I got it. However I have not checked to
select the other radio button what seems to be still the problem in this bug.

Comment 19

[Donnie Cranford]

racham,

Since we know this is a big problem especially since it basically destroys the
use of mail/news setup for new unfamiliar users can we get a nomination for 0.9.9?
Syd seems to concur with this idea. Thanks

-dcran-

Comment 20

[racham]

Attachment: patch, v1.1 - updated patch fixing the problem in the above snapshot

Needed to remove the initialization code, which once was introduced to make
default selection of the radio item, in order to allow the radiogroup element
to respond to user's actions in changing the choice (to newsaccount, etc).

I will move the milestone back to 099 and try to get this one in on both branch
and trunk.

Time to get reviews..

Blake, Hewitt, Can you do r & sr ? thanks. I have removed the Init() function,
as can be seen in the patch, as we are not doing the selection from js code
anymore. I had to add setAccountTypeData() in unload, as it is only by then
correct value for radiogroup.selectedItem is getting returned. Let me know if
you have more questions. I will send r & sr requests in mail separately.

Comment 21

[racham]

We should get this in on both branch & trunk. Changing the milestone to 099.

Comment 22

[racham]

*** Bug 129145 has been marked as a duplicate of this bug. ***

Comment 23

[Blake Ross]

var selectedItemId = rg.selectedItem.getAttribute('id');

Instead of .getAttribute('id');, please use .id

Why only now do we have to call setAccountTypeData from the unload?  How were we
previously doing it?  (I see that we did it in Init, but what happened if the
user changed the selection...?)

Comment 24

[racham]

Thanks.

Will change it to use '.id'.

Appropriate routines are called from oncommnd handlers (see below). So, if user
clicks on one of the radio items, it will taken care of via those handlers.

handlers :

1. Mail Accounts :
http://lxr.mozilla.org/mozilla/source/mailnews/base/prefs/resources/content/AccountWizard.xul#56

2. News Accounts :
http://lxr.mozilla.org/mozilla/source/mailnews/base/prefs/resources/content/AccountWizard.xul#73

Comment 25

[Blake Ross]

Ah, ok. So since we're doing it when you leave the page now (which seems right
to me anyway), can we remove those radio command handlers?

Comment 26

[racham]

Attachment: patch, v1.2 - updated with Blake's comments

Blake, here is the new patch. Please review when you get a chance. thanks.

Comment 27

[Blake Ross]

Comment on attachment 72690 [details] [diff] [review]
patch, v1.2 - updated with Blake's comments

sr=blake if you remove the dump in acctTypePageUnload() while you're there.

Comment 28

[racham]

Attachment: patch v1.3 - removed the dump statement

Comment 29

[racham]

Comment on attachment 72692 [details] [diff] [review]
patch v1.3 - removed the dump statement

c/f sr=blake.

Comment 30

[Donnie Cranford]

If this has driver approval lets go ahead and get this in....
Im trying to get my Solaris 0.9.9 builds built for testing and this is
prohibiting  mail / news testing.

Comment 31

[Asa Dotzler [:asa]]

drivers would like to see this land on the 0.9.9 branch. If it gets the
necessary review today it could still make it. 

Comment 32

[racham]

Sure. I planned on requesting driver's approval for this one as soon as r= is
stamped. I will ping Hewitt again and get that done. I will get a r= (from
Hewitt, ot from others if he is busy) and will get this in today (after a=).

Comment 33

[Joe Hewitt (gone)]

Comment on attachment 72692 [details] [diff] [review]
patch v1.3 - removed the dump statement

r=hewitt

Comment 34

[Asa Dotzler [:asa]]

Comment on attachment 72692 [details] [diff] [review]
patch v1.3 - removed the dump statement

a=asa (on behalf of drivers) for checkin to the 0.9.9 branch and the 1.0 trunk

Comment 35

[racham]

Fix checked in onto both 099 branch and trunk.

Comment 36

[André Dahlqvist]

VERIFIED FIXED with 2002-03-28-08 on Linux.