Bug 24079

Summary: REGRESSION(r39549): Page loads cannot be interrupted with Command-. or Escape
Product: WebKit Reporter: Jonas Walldén <jonasw>
Component: Page LoadingAssignee: 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 Flags
Backtrace of [WebView stopLoading:] as in r39548
none
patch darin: review+

Description Jonas Walldén 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.)
Comment 1 Mark Rowe (bdash) 2009-02-22 02:47:54 PST
<rdar://problem/6611233>
Comment 2 Mark Rowe (bdash) 2009-02-26 05:32:21 PST
Cameron, you broke this with <http://trac.webkit.org/changeset/39549>.
Comment 3 Cameron Zwarich (cpst) 2009-02-26 07:35:10 PST
Damn, I suck. I'll try to fix it.
Comment 4 Cameron Zwarich (cpst) 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.
Comment 5 Cameron Zwarich (cpst) 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:].
Comment 6 Adele Peterson 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.
Comment 7 Darin Adler 2009-03-05 17:15:32 PST
Comment on attachment 28339 [details]
patch

r=me

Testing is the key.
Comment 8 Adele Peterson 2009-03-05 17:43:48 PST
Committed revision 41467.