302424 - dialog.xml doButtonCommand

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Doug Turner (:dougt)]

currently doButtonCommand uses a timeout to avoid closing the window during the
oncommand event.  this self described hack is busting me on Minimo.  It seams
that with this the way it is, the dialog never is closed.  However, when I call
_reallyDoButtonCommand directly from _doButtonCommand (avoiding the timeout),
dialogs work.  I have tested window.confirm(), window.alert(), and
window.prompt()  with the following patch on Minimo.  This area of code hasn't
changed since blakeros landed it on 13-Oct-02.

I am not sure if this hack is still needed for all platforms, but it isn't for
WindowCE.

btw, i also tested window.setTimeout on Minimo and it works fine.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 2

[Doug Turner (:dougt)]

we may want to add WINCE to the #ifdef that XML can see.  I am not sure how to
do that.  I tried s/MINIMO/WINCE in the patch, and that didn't work.

Comment 3

[Benjamin Smedberg]

Comment on attachment 190782 [details] [diff] [review]
patch v.1

I believe that this code formerly existed to prevent crashes related to the
ordering of XBL constructors and JS scopes and such. If this actually works
now, we should do it for all platforms.

Comment 4

[Doug Turner (:dougt)]

i tested window.confirm(), window.alert(), and
window.prompt() on DeerPark Alpha 2 with a patch that removes this timeout and I
am not seeing any problems.  I have also played with some of the option dialogs
and don't see any issues.  Do we have any test cases?

Comment 5

[Doug Turner (:dougt)]

Attachment: patch v.2 -- removes timeout XP

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 190788 [details] [diff] [review]
patch v.2 -- removes timeout XP

I can't find why this was done, it was three years ago and the world has
changed since then.  If it works and hasn't been used in seamonkey and they
don't crash, it should be safe.

Comment 7

[Benjamin Smedberg]

Comment on attachment 190788 [details] [diff] [review]
patch v.2 -- removes timeout XP

I'm interested in dbaron's opinion on whether we should take this for 1.8

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 190788 [details] [diff] [review]
patch v.2 -- removes timeout XP

I don't have much of an opinion on this (and I'm not sure why you'd think I
would), as long as it doesn't cause some nasty stack where we break some of the
basic invariants in Gecko for which we don't have assertions (e.g., don't
modify the content tree or frame tree in the middle of reflow or repaint).

Comment 9

[Doug Turner (:dougt)]

Checking in dialog.xml;
/cvsroot/mozilla/toolkit/content/widgets/dialog.xml,v  <--  dialog.xml
new revision: 1.25; previous revision: 1.24
done

Comment 10

[Doug Turner (:dougt)]

marking fixed.