Add checkboxes to preview dialog

VERIFIED FIXED

Status

()

Toolkit
Form Manager
VERIFIED FIXED
17 years ago
9 years ago

People

(Reporter: Stephen P. Morse, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
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.
(Reporter)

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0
(Reporter)

Updated

17 years ago
Target Milestone: mozilla1.0 → M9
(Reporter)

Comment 1

17 years ago
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.
(Reporter)

Comment 2

17 years ago
Created attachment 28330 [details] [diff] [review]
patch to fix this bug and bug 67389
(Reporter)

Comment 3

17 years ago
cc'ing blakeross for code review
(Reporter)

Comment 4

17 years ago
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.
(Reporter)

Comment 5

17 years ago
Created attachment 28373 [details] [diff] [review]
correct for problem when an entry is not checked

Comment 6

17 years ago
r:pnunn
(Reporter)

Comment 7

17 years ago
cc'ing alecf for super review

Comment 8

17 years ago
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.
(Reporter)

Comment 9

17 years ago
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

17 years ago
let's just fix it now... please don't continue bad patterns
(Reporter)

Comment 11

17 years ago
Created attachment 28402 [details] [diff] [review]
same patch but using a .dtd file as requested
(Reporter)

Comment 12

17 years ago
Created attachment 28407 [details]
The dtd file

Comment 13

17 years ago
ok, looks good as long as you actually remove WalletPreview.properties from CVS,
not just the build.
sr=alecf
(Reporter)

Updated

17 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 14

17 years ago
Fix checked in

Comment 15

17 years ago
Verified w2k build 2001060409
Verified linux build 2001060506
Verified mac build 2001060508
Status: RESOLVED → VERIFIED
Assignee: morse → nobody
Component: Form Manager → Form Manager
Product: Core → Toolkit
QA Contact: tpreston → form.manager
Target Milestone: M9 → ---
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.