67379 - Add checkboxes to preview dialog

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect)

Description

[Stephen P. Morse]

Currently the form-manager preview dialog shows a list of values that can be 
prefilled for each field.  The last value is <do no prefill>.  Instead of doing 
that, each entry will be preceded by a checkbox which is originally checked -- 
the user unchecks the ones he does not want prefilled.

Comment 1

[Stephen P. Morse]

This bug involves the same files as bug 67389 (fields in preview menu to be 
editable) so I'll post one patch for both.  Also German's spec for the 
preview dialog involved adding a button to invoke the form manager so I'll 
do that here as well.

Attaching patch to fix all the above and do some minor housecleaning as well.

Comment 2

[Stephen P. Morse]

Attachment: patch to fix this bug and bug 67389

Comment 3

[Stephen P. Morse]

cc'ing blakeross for code review

Comment 4

[Stephen P. Morse]

Oops, bug in attached patch.  If you uncheck one of the checkboxes and then 
click OK, you get a javascript error and dialog is not dismissed.

Turns out one of the last changes I made was to disable the associated menulist 
when its checkbox is unchecked.  And I guess I never tested pressing OK after I 
made that change (prior to that change, an unchecked entry worked fine).

What happened is that the act of disabling a menulist causes the selected 
attribute to be unset.  Don't know if that is a bug or a feature.  Anyway, I can 
work around that by simply saving the value of the selected attribute before 
disabling and then restoring that attribute after disabling.

Attaching modified patch to correct this problem.

Comment 5

[Stephen P. Morse]

Attachment: correct for problem when an entry is not checked

Comment 6

[pnunn]

r:pnunn

Comment 7

[Stephen P. Morse]

cc'ing alecf for super review

Comment 8

[Blake Ross]

DTD's are the standard way they we store static strings used as xul labels.  I 
know you said you didn't want to add a dtd for one entity, but we may have more 
in the future, and the precedent you're setting now will be followed in the 
future for this file ("hmm...I don't want to create a dtd just for two 
entities...I'll obtain the string from a bundle and set it programmatically"). 
I would much prefer to see a dtd used here.

Comment 9

[Stephen P. Morse]

Blake, I agree.  There alread are several items that are initialized this way 
so I just filed a separate bug report on that (bug 72855).

Comment 10

[Alec Flett]

let's just fix it now... please don't continue bad patterns

Comment 11

[Stephen P. Morse]

Attachment: same patch but using a .dtd file as requested

Comment 12

[Stephen P. Morse]

Attachment: The dtd file

Comment 13

[Alec Flett]

ok, looks good as long as you actually remove WalletPreview.properties from CVS,
not just the build.
sr=alecf

Comment 14

[Stephen P. Morse]

Fix checked in

Comment 15

[Terri Preston]

Verified w2k build 2001060409
Verified linux build 2001060506
Verified mac build 2001060508