Bug 12462 – Need -[WebView setAllowsUndo:] API

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Jim Correia]

I have a webview in which editing actions should *not* be undoable. 

Returning nil from the delegate method:

    - (NSUndoManager *)undoManagerForWebView:(WebView *)webView;

is not sufficient to prevent undo registration, since the web view will still "find" the window's undo manager (which is used by other views in the window.)

NSTextView solves this problem with

-allowsUndo
-setAllowsUndo

Path attached which implements this.

[Jim Correia]

Created attachment 12747 [details]
Patch which implements the requested feature.

[David Kilzer (:ddkilzer)]

Comment on attachment 12747 [details]
Patch which implements the requested feature.

Jim, if you'd like this patch included in WebKit, you'll need to set the review? flag so that it will be looked at by a reviewer.  (I just did this to kick start the process.)

Also, you will need a ChangeLog entry with the patch.  (Use the WebKitTools/Scripts/prepare-ChangeLog script to do that; the instructions on the web need a bit more information.)

http://webkit.org/coding/contributing.html

[David Kilzer (:ddkilzer)]

How is this bug related to Bug 12461?

[Jim Correia]

Created attachment 12752 [details]
ChangeLog entry for previous patch.

[Jim Correia]

(In reply to comment #3)
> How is this bug related to Bug 12461?

It really isn't. 

This solves a different problem. The problem solved by this patch is when you have an editable webview in a window, but need to prevent any editing action in that webview from registering undo actions with the undo manager which is used by other views in that window (whose editing actions you do want registered.)

-allowsUndo/-setAllowsUndo: as implemented in the patch is the same mechanism NSTextView uses to solve the same problem.

[David Kilzer (:ddkilzer)]

Comment on attachment 12752 [details]
ChangeLog entry for previous patch.

Only approved code reviewers should set the review+ flag.  Please set the "review?" flag to request that the patch be reviewed.

[David Kilzer (:ddkilzer)]

Comment on attachment 12747 [details]
Patch which implements the requested feature.

Sorry, I thought the changelog attachment contained the whole patch again.  Resetting flags.

[Maciej Stachowiak]

Comment on attachment 12747 [details]
Patch which implements the requested feature.

This seems like a fine idea for an API addition. However, we cannot just add new API methods to the public header. To become public API, they need to go through API review. Please add them as SPI for now to the WebViewPrivate header in the PendingPublic category.

[Maciej Stachowiak]

Comment on attachment 12752 [details]
ChangeLog entry for previous patch.

ChangeLog looks fine, but r-'ing it along with the patch - please include in next patch.

[Jim Correia]

Created attachment 12766 [details]
patch

[Jim Correia]

Created attachment 12767 [details]
revised changelog

[Mark Rowe (bdash)]

Comment on attachment 12766 [details]
patch

A few comments:
1) you should include the ChangeLog entry as part of the patch rather than as a separate file.
2) +        if ([decoder containsValueForKey: @"AllowsUndo"])  has extra whitespace after the colon.
3) 
+    if (! _private->allowsUndo) {
+        return nil;
+    }

has unneeded braces.

If you could address those two minor issues, resubmit your patch and mark it r? I will take another look and land it.

[Jim Correia]

Created attachment 12770 [details]
revised patch

[Mark Rowe (bdash)]

Comment on attachment 12770 [details]
revised patch

r=me

[Mark Rowe (bdash)]

Landed in r19243 as SPI.