Summary: | REGRESSION(r39549): Page loads cannot be interrupted with Command-. or Escape | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jonas Walldén <jonasw> | ||||||
Component: | Page Loading | Assignee: | Adele Peterson <adele> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Minor | CC: | ap, zwarich | ||||||
Priority: | P2 | Keywords: | InRadar, Regression | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Jonas Walldén
2009-02-22 01:49:25 PST
Cameron, you broke this with <http://trac.webkit.org/changeset/39549>. Damn, I suck. I'll try to fix it. 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. 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:]. 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.
Comment on attachment 28339 [details]
patch
r=me
Testing is the key.
Committed revision 41467. |