[Webkit-unassigned] [Bug 8516] frame load delegate method webView:willCloseFrame: is called at the wrong times

bugzilla-daemon at opendarwin.org bugzilla-daemon at opendarwin.org
Fri Apr 21 08:45:10 PDT 2006


http://bugzilla.opendarwin.org/show_bug.cgi?id=8516


ggaren at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #7870|review?                     |review-
               Flag|                            |




------- Comment #4 from ggaren at apple.com  2006-04-21 08:45 PDT -------
(From update of attachment 7870)
Generally, I think you're right on here, but I have to r- for a few reasons:

1. Standard patch "etiquette" requires (1) a test case*; (2) changelog and
patch in same file; (3) adding your name to the copyrights of files you've
changed. 

2. Like you point out, I think it's wrong for _closeOldDataSources to send the
-willCloseFrame delegate call. The documentation says, "Invoked when a frame
will be closed," but _closeOldDataSources is called from
_commitProvisionalLoad, which makes the behavior, "Invoked when a frame loads
content." (However, I think it's right for _closeOldDataSources to call
-setMainFrameDocumentReady:NO.)

I sure wouldn't mind it if your patch renamed _detachFromParent to, say,
"_close."

* Test case: You can find examples in LayoutTests/fast. They're snippets of
HTML that demonstrate the problem. run-webkit-tests loads each snippet into a
mini-app called DumpRenderTree to dump the result. Since you're testing the
Objc bindings, you'll probably want to modify DumpRenderTree itself so that a
test can, through JavaScript, tell DumpRenderTree to register for the
-willCloseFrame delegate, then close a frame, then query DumpRenderTree to see
if got the delegate callback.


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list