When you have an editing delegate set on an editable webview, the webViewDidBeginEditing and
webViewDidEndEditing notification methods don't get called on the delegate.
1) Run my attached modified "blot" application
2) Click on an editable block, notice what editing delegate methods get logged
3) Click outside an editable block; note again what gets logged
Expected: You should see indication in the long of webViewDidBeginEditing: and webViewDidEndEditing:
Actual: You don't, though you see other editing delegate methods get logged.
Created attachment 2943 [details]
Modified BLOT sample app that logs all editing delegate methods
Graham Dennis had these comments which he allowed me to paste in: [edited for clarity]
Initially, I thought #3982 should be a quick fix, but it turns out that they only did the easy part by
putting in the support code on the WebKit side, but WebCore doesn't support the notifications. The
difficulty in fixing this depends on exactly when didBeginEditing/didEndEditing should be called
(shouldEndEditingInDOMRange works in ToT). Should didBeginEditing be called on the first edit in the
entire WebKit and didEndEditing be called when focus leaves the WebKit, or should it happen when
focus leaves an editable block, and a new didBeginEditing be called when a new editable block is
edited? [Dan -- I'd think the latter: Documentation says "when the user [begins|stops] editing the web
view"] For the former (and less useful) case, it should be fairly easy. The latter case would require
modifying the definition of KWQKHTMLPart::respondToChangedContents to give it another argument,
so it can determine if a new editable block has been edited. The long and the short of it is that this
could probably be fixed in a reasonable timeframe.
It seems to me that the didEndEditing notification should be sent after the editing delegate says YES to
shouldEndEditingInDOMRange. The documentation for shouldEndEditingInDOMRange says:
This method returns YES if the user should be allowed to end editing webView, NO otherwise. This
method is invoked when a web view attempts to resign as the first responder. Typically, the range
argument designates the current selection, although it might not. Use the range argument to help
determine if the user can end editing. If this method returns YES, webView will end editing and resign
as the first responder.
However this isn't what WebKit does. The shouldEndEditingInDOMRange method is called every time an
editable block loses focus. Likewise shouldBeginEditingInDOMRange is called every time an editable
block gains focus. Also note that shouldEndEditingInDOMRange doesn't consider whether the editable
block was edited while it was focused. Is this the behavior developers want? If so, than implementing
didBegin/didEnd should be straightforward, just add didBegin/didEnd calls after shouldBegin/
shouldEnd. If it isn't the behavior we want, we should fix it.
Darin says that we want these delegate methods to be called when editable blocks gain/lose focus, so
should be straightforward to add didBeginEditing and didEndEditing.
Created attachment 5934 [details]
With this patch, webViewDidBeginEditing and webViewDidEndEditing should now be called whenever the focus enters/leaves a contentEditable block. I had to define a whole bunch of functions for this, and I tried to follow the behaviour of shouldBeginEditing where appropriate.
Comment on attachment 5934 [details]
looks good, r=me
I have to make a few changes to DumpRenderTree in order to add test coverage for this.
Created attachment 6324 [details]
patch + changes to allow automated testing
After applying Graham's patch, I noticed that very few of the editing tests' results showed shouldBegin, shouldEnd, didBegin and didEndEditing delegate calls. They weren't being called because:
They're only called when contenteditable regions gain/lose focus.
Editing tests don't explicitly focus editable regions they are about to do editing in, they simply set selections inside them.
Setting selections inside contenteditable regions normally has the side effect of focusing that region, _except_ when the WebView was isn't focused.
DumpRenderTree's WebView isn't focused.
Tim and I don't think there's a way to make the WebView focused without also making it visible and inside a frontmost window, which would prevent you from doing useful work while you run the layout tests. So, I made the two calls made on the bridge into WebHTMLView SPI:
_setWindowHasFocus, which tells WebCore if the window containing the WebView is key. This will fire window.onfocus and window.onblur handlers.
_setDisplaysWithFocusAttributes, which tells WebCore that the WebView is in the responder chain, and should therefore do focusy things, like paint the caret, focus halos, and a selection with a normal tint.
Three things to note:
As I mentioned above, setting a selection inside a content editable region has the side effect of focusing that region. Adele and I don't think that this is the behavior we want. Discussion on that issue is at 7128.
This makes focus halos and carets show up in the pixel results, hurray.
There are mac line endings in some of my layout tests. I'm removing them.
Comment on attachment 6324 [details]
patch + changes to allow automated testing
Justin landed this in r12683.
And r12685 (and fixes to layout tests in r12701-r12711?).
Yes. I know, worst check in skills ever.