Bug 3734 – DumpRenderTree should report editing delegate operations, for regression checking

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Duncan Wilcox]

With the attached patch DumpRenderTree reports editing delegate method calls:

.../DumpRenderTree .../inserting/insert-3907422-fix.html
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: shouldDeleteDOMRange:range from 0 of #text > DIV > BODY > HTML > 
#document to 3 of #text > DIV > BODY > HTML > #document
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
layer at (0,0) size 800x600
  RenderCanvas at (0,0) size 800x600
layer at (0,0) size 800x600
  RenderBlock {HTML} at (0,0) size 800x600
    RenderBody {BODY} at (8,8) size 784x584
      RenderBlock {DIV} at (0,0) size 784x18
        RenderBR {BR} at (0,0) size 0x18
selection is CARET:
start:      position 0 of child 1 {BR} of child 1 {DIV} of root {BODY}
upstream:   position 0 of child 1 {DIV} of root {BODY}
downstream: position 0 of child 1 {BR} of child 1 {DIV} of root {BODY}

very few tests trigger webView:should* calls, that's why I picked this one in particular.

[Duncan Wilcox]

Created attachment 2671 [details]
DumpRenderTree patch

[Duncan Wilcox]

Comment on attachment 2671 [details]
DumpRenderTree patch

Checking this in will break all tests in WebCore/layout-tests/editing.

[Joost de Valk (AlthA)]

Seems reasonable.

[Darin Adler]

An alternative is to have the editing delegate dumping controlled by a layout test controller method, so it 
happens only if you request it.

But I think I like this as-is.

[Darin Adler]

Comment on attachment 2671 [details]
DumpRenderTree patch

This patch has code indented by 8 spaces rather than 4 spaces in it.

Otherwise, looks great.

[Duncan Wilcox]

Created attachment 2686 [details]
DumpRenderTree patch

Ooops, 4 spaces now.

[Darin Adler]

Comment on attachment 2686 [details]
DumpRenderTree patch

Looks good, r=me

[Darin Adler]

Comment on attachment 2686 [details]
DumpRenderTree patch

My testing shows that this makes the a few layout tests fail because their
result is influenced by the previous layout test.

If the previous test caused a selection, then the next test will have this:

EDITING DELEGATE:
webViewDidChangeSelection:WebViewDidChangeSelectionNotification

We'll have to resolve this before landing the patch.

[Duncan Wilcox]

What is happening is that loading a new document (loadRequest:) clears the existing document that 
trickles all the way to KHTMLPart::clearSelection(), that leads to the notification.

I'm not sure this isn't the correct behaviour, however I see how it is a problem in the context of testing.

If notifications should be suppressed when loading a new document then I have developed a patch for 
that.

If DumpRenderTree should prevent notifications on its own, isolating one test from the next, I can write a 
patch that releases the webview and recreates a new one for every test.

[Duncan Wilcox]

Created attachment 2743 [details]
patch with changelog

This patch includes the previous changes plus explicitly clearing the selection
after each test. The selection clearing causes a webViewDidChangeSelection
notification that is not logged by checking the global "done" flag that signals
in-progress tests.

[Maciej Stachowiak]

Comment on attachment 2743 [details]
patch with changelog

r=me assuming this doesn't affect anything besides the editing tests where it
should add the delegate logging.