314306 - [patch] Can't quit MODI / something's eating Esc in the content area

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect, P3)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

When using the Mouseover Document Inspector bookmarklet (above; homepage: http://slayeroffice.com/tools/modi/v2.0/modi_help.html), Esc is supposed to cancel the bookmarklet and make it go away (and does, in Fx).  

In Camino, Esc does not cancel MODI.  I suspect something is eating Esc (the other shortcut keys, like p for Pause, do work); bug 310989 seems to indicate Esc gets grabbed somewhere.

Camino 2005102704 (v1.0a1+)

Comment 1

[Simon Fraser [no longer active]]

Neat bookmarklet! We should look into this.

Comment 2

[Wevah (Nate Weaver)]

Esc stops page load. I think that we should only be trapping Esc for this when the page actually is loading, though.

Comment 3

[Simon Fraser [no longer active]]

Taking.

Comment 4

[Simon Fraser [no longer active]]

Attachment: Patch

The whole -sendEvent thing was a bad idea. The correct way to do this is to implement the NSResponder method -cancel:, which is called when you hit the escape key. That way, we can implement it at the appropriate place in the responder chain.

Comment 5

[Wevah (Nate Weaver)]

*** Bug 319161 has been marked as a duplicate of this bug. ***

Comment 6

[Mark Mentovai]

Comment on attachment 204995 [details] [diff] [review]
Patch

Nice.  That bookmarklet is really cool.

Why cancel: and not cancelOperation:?

Comment 7

[Simon Fraser [no longer active]]

I used cancel because that's what I saw doComamndBySelector: getting called for down in ChildView. If we implement cancelOperation:, I'm not sure that gecko's gonna get first crack at it.

Comment 8

[Mark Mentovai]

Comment on attachment 204995 [details] [diff] [review]
Patch

Both cancel: and cancelOperation: seemed to do the trick for me, I was just curious about your choice of one over the other.  r=me.

Comment 9

[Simon Fraser [no longer active]]

Unfortunately we have to have a solution that doesn't break 10.2 (like keeping the old sendEvent: hack).

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Is bug 289432 related to this (i.e., to our trapping of certain keystrokes at impropper occasions?)

Comment 11

[Wevah (Nate Weaver)]

I think that's just because we're not catching it in the sheet, but we're catching it for the "Stop Loading Page" menu item.

Comment 12

[Chris Lawson (gone)]

Tangentially related: bug 164863, which describes a situation where cmd gets eaten too. (For example, cmd-return won't open a text-selected link at all, let alone in a new tab or window. Return with the link selected works as expected.)

cl

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Simon, should we move this to 1.1/Mozilla 1.8.1 (when we won't have to make it work for 10.2)?

Comment 14

[Simon Fraser [no longer active]]

Yeah I guess.

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 204995 [details] [diff] [review]
Patch

This still applies, but it has to guess some offsets now.  It'd be nice to get it in soon ;)

Comment 16

[Mark Mentovai]

Comment on attachment 204995 [details] [diff] [review]
Patch

10.2-dropping OK for 1.1.

Comment 17

[Mark Mentovai]

Checked in on trunk and 1.8 branch for smfr.

Comment 18

[Stuart Morgan]

*** Bug 339811 has been marked as a duplicate of this bug. ***