Bug 139408 - [iOS] Safari crashes at -[_NSUndoStack popAndInvoke] when trying to undo typing on closed tab
Summary: [iOS] Safari crashes at -[_NSUndoStack popAndInvoke] when trying to undo typi...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: mitz
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-12-08 11:28 PST by mitz
Modified: 2014-12-08 13:53 PST (History)
1 user (show)

See Also:


Attachments
Give each WKWebContentView an undo manager (1.92 KB, patch)
2014-12-08 11:34 PST, mitz
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2014-12-08 11:28:51 PST
<rdar://problem/18046692>

Steps to Reproduce:
1. Type something in an input field on a webpage
2. Enter tab view, and close the tab you just typed in
3. Go to another tab
4. Shake to bring up the Undo Typing dialog
5. Tap Undo

Result:
Crash beneath -[_NSUndoStack popAndInvoke].
Comment 1 mitz 2014-12-08 11:34:39 PST
Created attachment 242834 [details]
Give each WKWebContentView an undo manager
Comment 2 Anders Carlsson 2014-12-08 11:38:07 PST
Should we do this on OS X as well? Should we do it on the WKWebVIew level instead?
Comment 3 mitz 2014-12-08 11:46:01 PST
(In reply to comment #2)
> Should we do this on OS X as well?

I don’t know if this bug exists in OS X. The Legacy WebKit implementation of -undoManager differs between iOS and OS X.

> Should we do it on the WKWebVIew level instead?

I don’t know. This change appears to fix the bug as it is.
Comment 4 Anders Carlsson 2014-12-08 11:48:02 PST
(In reply to comment #3)

> > Should we do it on the WKWebVIew level instead?
> 
> I don’t know. This change appears to fix the bug as it is.

OK. Do you think adding it to the content view instead of the WKWebView could cause any problems for people subclassing WKWebView and overriding -undoManager?
Comment 5 mitz 2014-12-08 11:49:59 PST
(In reply to comment #4)
> (In reply to comment #3)
> 
> > > Should we do it on the WKWebVIew level instead?
> > 
> > I don’t know. This change appears to fix the bug as it is.
> 
> OK. Do you think adding it to the content view instead of the WKWebView
> could cause any problems for people subclassing WKWebView and overriding
> -undoManager?

Functions in PageClientImplIOS.mm access the WKContentView’s undoManager property directly, so the implementation in the WKWebView subclass won’t be called.
Comment 6 mitz 2014-12-08 13:53:30 PST
Fixed in <http://trac.webkit.org/r176969>.