116960 - Form manager preview window stops appearing

Status: VERIFIED FIXED

(Toolkit :: Form Manager, defect)

Description

[Stephen P. Morse]

If you go to prefill a form, then decide to cancel out of it by clicking on the 
X in the upper right corner of the dialog, you will not be able to prefill any 
more forms for the remainder of the browser session.

To demonstrate:

1. Go to any form
2. Fill in any field, then do edit->save-form-data

<<step 2 was just to get some value saved so that prefilling will be possible>>

3. Start to prefill the form by doing edit->prefill-form
4. A preview dialog appears showing the values that will be prefilled
5. Cancel by clicking on the X in the upper right of the preview dialog
6. Repeat step 3 again

expected: the preview dialog should appear
acutal: the preview dialog never appears again during that browser session.

Comment 1

[Stephen P. Morse]

Problem is that the "cancel" handler is not called when clicking on the X but it 
is called when clicking on the cancel button.  Definitely a bug in xul.  However 
the problem can be corrected if the dialog is changed from a <window> to a 
<dialog>.  Attaching a patch that makes this change.

Comment 2

[Stephen P. Morse]

Attachment: convert from <window> to <dialog>

Comment 3

[Stephen P. Morse]

cc'ing sgehani and alecf for reviews

Comment 4

[Samir Gehani]

Comment on attachment 62777 [details] [diff] [review]
convert from <window> to <dialog>

>+<dialog title="&windowtitle.label;"
>         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>+        ondialogaccept="return Save();"
>+        ondialogcancel="return Cancel();"
>+        buttonpack="center"

Is there a reason to deviate from the dialog XBL default of buttonpacking?

>-    <hbox id="okCancelButtons"/>
>+    <button dlgtype="accept" class="dialog-button"/>
>+    <button dlgtype="cancel" class="dialog-button"/>

The dialog XBL includes these buttons for you.	Is there some reason you are
including these buttons explicitly?

Comment 5

[Stephen P. Morse]

> Is there a reason to deviate from the dialog XBL default of buttonpacking?

No reason.  I blindly copied the buttonpack=center line from a patch that hewitt 
has for bug 114580.  But I noticed that if I remove that line in my patch, it 
makes no difference.  So consider that the line is removed from the patch in 
this bug report.

> The dialog XBL includes these buttons for you.  Is there some reason you are
> including these buttons explicitly?

This dialog has three buttons on the bottom line -- a view button on the left 
and the OK/cancel buttons on the right.  If I didn't explicitly include the 
OK/cancel buttons in the dialog, those buttons would appear on a line below the 
view button rather than on the same line.

Comment 6

[Samir Gehani]

Comment on attachment 62777 [details] [diff] [review]
convert from <window> to <dialog>

r=sgehani with buttonpack removed.

Comment 7

[Alec Flett]

Comment on attachment 62777 [details] [diff] [review]
convert from <window> to <dialog>

ok, what samir said. sr=alecf

Comment 8

[Stephen P. Morse]

Patch without buttonpack checked in.

Comment 9

[Terri Preston]

Verified fixed Win xp build 2002032803