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)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED DUPLICATE of bug 93732
mozilla0.9.5

People

(Reporter: TucsonTester2, Assigned: cmanske)

Details

(Whiteboard: EDITORBASE (1/2 day))

Attachments

(1 file)

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
confirmed, error in console: " Warning prev sibling is not in our list!!! "
Status: UNCONFIRMED → NEW
Ever confirmed: true
-->cmanske
Assignee: brade → cmanske
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.
Reproduce issue on win 95 4.00.950b ie 5.5 using build 2001082109
This needs some attention please! Doesn't seem like Chris should own this, imho.
Severity: normal → major
Target Milestone: --- → mozilla0.9.4
I think cmanske meant XPToolkit/Widgets.
Assignee: waterson → trudelle
Component: XP Miscellany → XP Toolkit/Widgets
QA Contact: brendan → jrgm
Thanks! Sorry about that.
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
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!
ready to review
Status: NEW → ASSIGNED
Keywords: patch, review
Whiteboard: FIX IN HAND need r=, sr=
r=jrgm
... 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 ...
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.
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=
sr=kin (reviewed at Charley's machine)
Whiteboard: FIX IN HAND need sr= → FIX IN HAND reviewed
Keywords: nsbranch+
a=asa on behalf of drivers
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
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
spam composer change
Component: Editor: Core → Editor: Composer
Whiteboard: EDITORBASE (1/2 day)
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
verified.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: