238391 - quit-application-requested should support cancelling

Status: RESOLVED WORKSFORME

(SeaMonkey :: UI Design, defect)

Description

[timeless]

Comment 1

[timeless]

Attachment: steal toolkit impl

Comment 2

[[e-mail]]

Um... of course, written like this, it also supports uncancelling :-P

Comment 3

[timeless]

the alternative would be to have a callback object where you could do:

actor->Cancel(refuser) so that the thing trying to quit could be told which
thing refused. but for the time being i'd like to do this.

Comment 4

[[e-mail]]

FWIW this way is more compatible with FF too ;-)

Comment 5

[[e-mail]]

Hmm... I think (not tried) you could break this as follows:
1. Observer 1, written in any language, sets cancelQuit to true.
2. Observer 2, written in JS, throws an exception.
3. When Observe returns the exception gets propagated out (for some reason).
4. The quit is not cancelled at Observer 1's request :-(

Comment 6

[timeless]

Attachment: honor toolkit api

Comment 7

[[e-mail]]

Comment on attachment 144750 [details] [diff] [review]
honor toolkit api

Surely this solves the wrong problem - to honour the (unwritten?) contract you
enumerate the observers and abort the loop as soon as one cancels.

Comment 8

[Frédéric Buclin]

Can we expect this bug to be solved before 1.8 final? This would be very useful
considering bug 28385.

Comment 9

[timeless]

Attachment: bail at first rejection

Comment 10

[timeless]

[e-mail]: if you're volunteering to fix this bug, then take
ownership and set flags. otherwise please leave it alone.

Comment 11

[[e-mail]]

Comment on attachment 146193 [details] [diff] [review]
bail at first rejection

>+    var observer;
>+    while (observers.hasMoreElements() &&
>+           (observer = observers.getNext()))
I'm not sure that quitting the loop if the getNext returns null is useful.

>+  catch (ex)
I would have thought having the catch inside the loop would be more reliable.

Comment 12

[[e-mail]]

Comment on attachment 146193 [details] [diff] [review]
bail at first rejection

Nits: 1) I don't think that a null getNext() should exit the loop, getNext
should throw on a serious error 2) I think that you should treat an exception
in the notification as a failure to cancel the quit rather than as a force
quit.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 146193 [details] [diff] [review]
bail at first rejection

+	 observer.notify(cancelQuit, "quit-application-requested", null);

sorry, that method is called observe.

also, I don't think an exception from an observer should cancel the quit. (or
should it?)

Comment 14

[Serge Gautherie (:sgautherie)]

timeless,
Are you still working on this ?

Comment 15

[Philip Chee]

> steal toolkit impl
we are now using Toolkit and toolkit's globalOverlay.js