RESOLVED FIXED 24079
REGRESSION(r39549): Page loads cannot be interrupted with Command-. or Escape
https://bugs.webkit.org/show_bug.cgi?id=24079
Summary REGRESSION(r39549): Page loads cannot be interrupted with Command-. or Escape
Jonas Walldén
Reported 2009-02-22 01:49:25 PST
The loading of a page cannot be interrupted with keyboard shortcuts such as the Escape key or Command-period. The "Stop" command in the the View menu and the stop button in the toolbar still work as expected. Note that this does not apply to the very first phases of page loading (probably DNS resolving or similar) but rather when the page data arrives and the window shows some of the page content. Tested with WebKit Nightly r41121, though the issue has been present for many weeks or even months. (Speaking of interrupting loading I'd like to suggest a Stop button per sub-request in the Activity window as well, but that should be a separate ticket.)
Attachments
Backtrace of [WebView stopLoading:] as in r39548 (2.56 KB, text/plain)
2009-02-26 21:07 PST, Cameron Zwarich (cpst)
no flags
patch (2.78 KB, patch)
2009-03-05 17:07 PST, Adele Peterson
darin: review+
Mark Rowe (bdash)
Comment 1 2009-02-22 02:47:54 PST
Mark Rowe (bdash)
Comment 2 2009-02-26 05:32:21 PST
Cameron, you broke this with <http://trac.webkit.org/changeset/39549>.
Cameron Zwarich (cpst)
Comment 3 2009-02-26 07:35:10 PST
Damn, I suck. I'll try to fix it.
Cameron Zwarich (cpst)
Comment 4 2009-02-26 20:56:49 PST
This problem is caused by a ocmbination of r38629 and r39549. After r38629, all key events pass through the editor machinery. Most key combinations in the browser are promptly ignored. However, escape and command-. give the "cancelOperation" editor command, which causes them to be handled by doCommandBySelector. Strangely, cancelOperation: works fine with [super doCommandBySelector:selector], but if we ignore it like my change in r38549 does, then it doesn't get handled correctly by the responder chain. I know that these events are handled in a weird way by AppKit, but I haven't looked into it much further. As an aside, it might be best to decide whether to handle editor events (even though we are not currently editing) outside of doCommandBySelector. I'll play around with this a bit to see if I can shed more light on the problem, but it is probably best if an Apple person can take this bug.
Cameron Zwarich (cpst)
Comment 5 2009-02-26 21:07:19 PST
Created attachment 28062 [details] Backtrace of [WebView stopLoading:] as in r39548 This is a backtrace of [WebView stopLoading:] before my change in r39549 (I just reverted locally). The event gets passed to Safari by [NSWindow doCommandBySelector:].
Adele Peterson
Comment 6 2009-03-05 17:07:31 PST
Created attachment 28339 [details] patch I tested this by making sure cmd-. and escape cause loading to be stopped. I tested that ctrl-tab works to switch tabs when focus is in a text field. And I tested that pageup/pagedown and home/end work with Mail. And I ran layout tests. I'm not convinced this solution is perfect, but I am sure it works better than our current code for many cases.
Darin Adler
Comment 7 2009-03-05 17:15:32 PST
Comment on attachment 28339 [details] patch r=me Testing is the key.
Adele Peterson
Comment 8 2009-03-05 17:43:48 PST
Committed revision 41467.
Note You need to log in before you can comment on or make changes to this bug.