Closed
Bug 96891
Opened 23 years ago
Closed 23 years ago
Closing dialog with "X" doesn't call the assigned "onCancel" method.
Categories
(SeaMonkey :: Composer, defect)
SeaMonkey
Composer
Tracking
(Not tracked)
mozilla0.9.5
People
(Reporter: TucsonTester2, Assigned: cmanske)
Details
(Whiteboard: EDITORBASE (1/2 day))
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0b; Windows NT 5.1)
BuildID: 20010821
If you open the characters and symbols menu and click on the close button to
close it, it can be reopened. However, if you opwn the menu and use the X at
the top right hand corner to close it then you cannot reopen it without
restarting composer.
Reproducible: Always
Steps to Reproduce:
1. Open a new composer page
2. Click on insert then choose Characters and Symbols
3. Click on the close button
4. Reopen the Characters and symbols menu
5. Click on the 'X' at the top to close it
6. Try to reopen the menu
Actual Results: Menu does not open
Expected Results: menu should open
Composer has to be restarted to open menu
Comment 1•23 years ago
|
||
confirmed, error in console:
" Warning prev sibling is not in our list!!! "
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 3•23 years ago
|
||
This worked fine in 6.1 and we haven't changed any Composer code.
I know there has been recent changes in the XPFE dialog code.
The Insert Character dialog is non-modal, so the parent window keeps track of
an existing child Insert Character window with the member variable:
"InsertCharWindow".
What is broke is that using the "X" to close a window doesn't call the usual
"Cancel" method. We set this method as we usually do with:
doSetOKCancel(onOK, Cancel);
in the dialog startup code. This is the Cancel method in EdInsertChars.js:
function Cancel()
{
window.opener.InsertCharWindow = null;
SaveWindowLocation();
window.close();
}
So something is broken in XP code. If this "Cancel" method isn't called,
the reference to the now-destroyed "InsertCharWindow" is not cleared, so
the command to open the dialog tries to find that window, but it doesn't exist.
I'd like this to be examined for 0.9.4, as the bug completely breaks the
Insert Character feature. I think there must be some other bad side effects
as well!
Assignee: cmanske → waterson
Component: Editor → XP Miscellany
Keywords: regression
QA Contact: sujay → brendan
Summary: Characters and symbols menu wont open → Closing non-modal dialog with "X" doesn't call the assigned "onCancel" method.
Reporter | ||
Comment 4•23 years ago
|
||
Reproduce issue on win 95 4.00.950b ie 5.5 using build 2001082109
Assignee | ||
Comment 5•23 years ago
|
||
This needs some attention please!
Doesn't seem like Chris should own this, imho.
Severity: normal → major
Target Milestone: --- → mozilla0.9.4
Comment 6•23 years ago
|
||
I think cmanske meant XPToolkit/Widgets.
Assignee: waterson → trudelle
Component: XP Miscellany → XP Toolkit/Widgets
QA Contact: brendan → jrgm
Assignee | ||
Comment 7•23 years ago
|
||
Thanks! Sorry about that.
Comment 8•23 years ago
|
||
Er, well, this didn't work in 6.1 (I just reproduced this defect in that
build).
At any rate, there is no implicit hookup of the "Cancel" button with any code
that will be run when the window is destroyed. (Maybe there should be, but that
is the way it has been thus far, and should be a separate bug from this one
if filed).
If there is something that must be handled no matter how a window is destroyed,
then it should be done in the unload handler of the window, e.g.:
@@ -33,6 +33,7 @@
xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
xmlns:html="http://www.w3.org/1999/xhtml"
onload = "Startup()"
+ onunload = "window.opener.InsertCharWindow = null;"
orient="vertical">
[You might also consider moving 'SaveWindowLocation();' into the unload
handler, but I don't know; I haven't looked at what it does].
(There is also an 'onclose' handler, that will allow you to entirely abort
the closing of the window, but that handler is not triggered in the case of
of a direct call to window.close().)
Assignee: trudelle → cmanske
Component: XP Toolkit/Widgets → Editor
QA Contact: jrgm → sujay
Assignee | ||
Comment 9•23 years ago
|
||
Darn! I thought I testing closing the window with "X" when I first implemented
this, but you're right, the problem does exist in 6.1!
Thanks for the help - what you suggest works great!
Assignee | ||
Comment 10•23 years ago
|
||
Assignee | ||
Comment 11•23 years ago
|
||
ready to review
Comment 12•23 years ago
|
||
r=jrgm
Comment 13•23 years ago
|
||
... although, noting that there is a generic onCancel() in EdDialogCommon.js,
I'm wondering if there might be other places that are assuming that certain
cleanup actions are taken whenever/however a window is closed ...
Assignee | ||
Comment 14•23 years ago
|
||
You are correct: most of Composer's dialogs use the generic "onCancel" method.
What is missed by using the "X" is just "SaveWindowLocation();", so we can
certainly live with not having that for the short term.
Note that we do hit that "onCancel" when the "Esc" key is used to cancel a
dialog.
Comment 15•23 years ago
|
||
please keep this bug open after you checkin to track the remaining dialog issues.
Whiteboard: FIX IN HAND need r=, sr= → FIX IN HAND need sr=
Assignee | ||
Comment 16•23 years ago
|
||
sr=kin (reviewed at Charley's machine)
Assignee | ||
Updated•23 years ago
|
Whiteboard: FIX IN HAND need sr= → FIX IN HAND reviewed
Comment 17•23 years ago
|
||
a=asa on behalf of drivers
Assignee | ||
Comment 18•23 years ago
|
||
Patch on 08/28/01 checked in, so the more important issue for the non-Modal
"Insert Character" dialog is fixed.
Keeping bug open to cover similar issue in all Composer dialogs:
We need to handle what we do in the common 'onCancel()' method for all dialogs
that use this common close method by adding appropriate "onunload" handlers.
Severity: major → normal
Keywords: nsbranch+,
patch,
regression,
review
Summary: Closing non-modal dialog with "X" doesn't call the assigned "onCancel" method. → Closing dialog with "X" doesn't call the assigned "onCancel" method.
Whiteboard: FIX IN HAND reviewed
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Assignee | ||
Updated•23 years ago
|
Whiteboard: EDITORBASE (1/2 day)
Assignee | ||
Comment 20•23 years ago
|
||
I'm trying to simplify my dialog bugs by coalescing all the very simple tasks
that touch multiple dialogs into one bug. This problem is not covered by
bug 93732.
*** This bug has been marked as a duplicate of 93732 ***
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → DUPLICATE
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•