Bug 110655 - Add willGoToHistoryItem on WebKit(1) Mac platform
Summary: Add willGoToHistoryItem on WebKit(1) Mac platform
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac All
: P2 Normal
Assignee: Alice Liu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-22 16:00 PST by Alice Liu
Modified: 2013-02-26 10:57 PST (History)
2 users (show)

See Also:


Attachments
patch (2.87 KB, patch)
2013-02-22 16:05 PST, Alice Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alice Liu 2013-02-22 16:00:40 PST
WebKit(1) Mac has shouldGoToHistoryItem but it doesn't have willGoToHistoryItem.  I'd like to add it.
Comment 1 Alice Liu 2013-02-22 16:05:32 PST
Created attachment 189855 [details]
patch
Comment 2 mitz 2013-02-25 00:15:48 PST
Comment on attachment 189855 [details]
patch

Is there any case where returning YES from -webView:shouldGoToHistoryItem: doesn’t mean that the WebView will go to that item? Can’t the delegate do whatever it could do in -webView:wellGoToHistoryItem: in its implementation of -webView:shouldGoToHistoryItem: just before returning YES?
Comment 3 Alice Liu 2013-02-25 16:31:33 PST
(In reply to comment #2)
> (From update of attachment 189855 [details])
> Is there any case where returning YES from -webView:shouldGoToHistoryItem: doesn’t mean that the WebView will go to that item? Can’t the delegate do whatever it could do in -webView:wellGoToHistoryItem: in its implementation of -webView:shouldGoToHistoryItem: just before returning YES?

In WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm, as currently written in the patch, there is no situation in which an affirmative "should-" doesn't also result in a subsequent call to "will-".  However, I don't necessary agree with the notion that simply because it doesn't means that we're better off combining the purpose of both into just "should-".  As I see it, the purpose of "should-doSomething" is to let the client have a say in whether doSomething should occur.  And naturally "will-doSomething" is to let the client know about forthcoming action.  Please see <a href="http://trac.webkit.org/browser/trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp#L957">WebKit2 WebFrameLoaderClient.cpp</a> for a similar example.
Comment 4 mitz 2013-02-25 16:41:27 PST
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 189855 [details] [details])
> > Is there any case where returning YES from -webView:shouldGoToHistoryItem: doesn’t mean that the WebView will go to that item? Can’t the delegate do whatever it could do in -webView:wellGoToHistoryItem: in its implementation of -webView:shouldGoToHistoryItem: just before returning YES?
> 
> In WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm, as currently written in the patch, there is no situation in which an affirmative "should-" doesn't also result in a subsequent call to "will-".  However, I don't necessary agree with the notion that simply because it doesn't means that we're better off combining the purpose of both into just "should-".  As I see it, the purpose of "should-doSomething" is to let the client have a say in whether doSomething should occur.  And naturally "will-doSomething" is to let the client know about forthcoming action.

Indeed. But in Cocoa API, when a “should” exists, a “will” is normally not necessary. See for example the NSTableViewDelegate method -tableView:shouldReorderColumn:toColumn: or the NSTextView delegate method -textView:shouldChangeTextInRange:replacementString:.

> Please see <a href="http://trac.webkit.org/browser/trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp#L957">WebKit2 WebFrameLoaderClient.cpp</a> for a similar example.

That’s quite different. The “should” call goes to the injected bundle client, whereas the “will” call goes to some client in the UI process. In your patch, you are sending the two messages to the same delegate.