Bug 3734 - DumpRenderTree should report editing delegate operations, for regression checking
Summary: DumpRenderTree should report editing delegate operations, for regression chec...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 412
Hardware: Mac OS X 10.4
: P3 Enhancement
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks: 4011
  Show dependency treegraph
 
Reported: 2005-06-27 07:52 PDT by Duncan Wilcox
Modified: 2005-07-31 19:31 PDT (History)
0 users

See Also:


Attachments
DumpRenderTree patch (6.22 KB, patch)
2005-06-27 07:54 PDT, Duncan Wilcox
darin: review-
Details | Formatted Diff | Diff
DumpRenderTree patch (5.98 KB, patch)
2005-06-28 06:37 PDT, Duncan Wilcox
darin: review-
Details | Formatted Diff | Diff
patch with changelog (8.18 KB, patch)
2005-07-01 21:43 PDT, Duncan Wilcox
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Duncan Wilcox 2005-06-27 07:52:34 PDT
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.
Comment 1 Duncan Wilcox 2005-06-27 07:54:29 PDT
Created attachment 2671 [details]
DumpRenderTree patch
Comment 2 Duncan Wilcox 2005-06-27 07:56:37 PDT
Comment on attachment 2671 [details]
DumpRenderTree patch

Checking this in will break all tests in WebCore/layout-tests/editing.
Comment 3 Joost de Valk (AlthA) 2005-06-27 09:46:11 PDT
Seems reasonable.
Comment 4 Darin Adler 2005-06-27 09:54:04 PDT
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.
Comment 5 Darin Adler 2005-06-27 16:40:54 PDT
Comment on attachment 2671 [details]
DumpRenderTree patch

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

Otherwise, looks great.
Comment 6 Duncan Wilcox 2005-06-28 06:37:57 PDT
Created attachment 2686 [details]
DumpRenderTree patch

Ooops, 4 spaces now.
Comment 7 Darin Adler 2005-06-28 10:43:46 PDT
Comment on attachment 2686 [details]
DumpRenderTree patch

Looks good, r=me
Comment 8 Darin Adler 2005-06-29 23:25:56 PDT
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.
Comment 9 Duncan Wilcox 2005-06-30 06:18:45 PDT
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.
Comment 10 Duncan Wilcox 2005-07-01 21:43:39 PDT
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.
Comment 11 Maciej Stachowiak 2005-07-02 23:55:06 PDT
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.