LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Mar 2006 06:38:22 +0000 (06:38 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Mar 2006 06:38:22 +0000 (06:38 +0000)
        Reviewed by darin

        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8067>
        REGRESSION: selectionRect includes next/previous replaced elements
        <rdar://problems/4402375&4474871&4474871>

        * editing/selection/4402375-expected.checksum: Added.
        * editing/selection/4402375-expected.png: Added.
        * editing/selection/4402375-expected.txt: Added.
        * editing/selection/4402375.html: Added.
        Turned on dumpSelectionRect for these tests:
        * editing/selection/replaced-boundaries-1-expected.checksum:
        * editing/selection/replaced-boundaries-1-expected.png:
        * editing/selection/replaced-boundaries-1.html:
        * editing/selection/replaced-boundaries-2-expected.checksum:
        * editing/selection/replaced-boundaries-2-expected.png:
        * editing/selection/replaced-boundaries-2.html:
        * editing/selection/replaced-boundaries-3-expected.checksum:
        * editing/selection/replaced-boundaries-3-expected.png:
        * editing/selection/replaced-boundaries-3.html:
        * editing/selection/image-before-linebreak-expected.checksum: Added.
        * editing/selection/image-before-linebreak-expected.png: Added.
        * editing/selection/image-before-linebreak-expected.txt: Added.
        * editing/selection/image-before-linebreak.html: Added.

WebCore:

        Reviewed by darin

        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8067>
        REGRESSION: selectionRect includes next/previous replaced elements
        Also fixes: <rdar://problems/4402375&4474871&4474871>

        In the case where a selection starts at the end or ends at the start
        of o, o->selectionState() != SelectionNone, but o isn't really selected.
        Constraining the selection with upstream and downstream eliminates these
        types of endpoints, but constraining endpoints that occur at the
        start or end of a paragraph creates positions inside containers - some
        of which the selection painting code isn't equipped to handle.

        * dom/Document.cpp:
        (WebCore::Document::updateSelection):
        * rendering/render_replaced.cpp:
        (WebCore::RenderReplaced::shouldPaint):
        (WebCore::RenderReplaced::selectionRect):
        (WebCore::RenderReplaced::setSelectionState):
        (WebCore::RenderWidget::setSelectionState):

WebKitTools:

        Reviewed by darin

        <rdar://problem/4402375>
        REGRESSION (417.8-TOT): selectionRect sometimes includes adjacent images

        Added an option to draw the selectionRect.

        * DumpRenderTree/DumpRenderTree.m:
        (dump):
        (+[LayoutTestController isSelectorExcludedFromWebScript:]):
        (-[LayoutTestController dumpSelectionRect]):
        (dumpRenderTree):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@13570 268f45cc-cd09-0410-ab3c-d52691b4dbfc

23 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/selection/4402375-expected.checksum [new file with mode: 0644]
LayoutTests/editing/selection/4402375-expected.png [new file with mode: 0644]
LayoutTests/editing/selection/4402375-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/4402375.html [new file with mode: 0644]
LayoutTests/editing/selection/image-before-linebreak-expected.checksum [new file with mode: 0644]
LayoutTests/editing/selection/image-before-linebreak-expected.png [new file with mode: 0644]
LayoutTests/editing/selection/image-before-linebreak-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/image-before-linebreak.html [new file with mode: 0644]
LayoutTests/editing/selection/replaced-boundaries-1-expected.checksum
LayoutTests/editing/selection/replaced-boundaries-1-expected.png
LayoutTests/editing/selection/replaced-boundaries-1.html
LayoutTests/editing/selection/replaced-boundaries-2-expected.checksum
LayoutTests/editing/selection/replaced-boundaries-2-expected.png
LayoutTests/editing/selection/replaced-boundaries-2.html
LayoutTests/editing/selection/replaced-boundaries-3-expected.checksum
LayoutTests/editing/selection/replaced-boundaries-3-expected.png
LayoutTests/editing/selection/replaced-boundaries-3.html
WebCore/ChangeLog
WebCore/dom/Document.cpp
WebCore/rendering/render_replaced.cpp
WebKitTools/ChangeLog
WebKitTools/DumpRenderTree/DumpRenderTree.m

index f732c068dab939aba84641b41fc06ca58f3c922d..96184317d9b899157765f24d4096c77b30effd2c 100644 (file)
@@ -1,3 +1,30 @@
+2006-03-29  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+        
+        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8067>
+        REGRESSION: selectionRect includes next/previous replaced elements
+        <rdar://problems/4402375&4474871&4474871>
+        
+        * editing/selection/4402375-expected.checksum: Added.
+        * editing/selection/4402375-expected.png: Added.
+        * editing/selection/4402375-expected.txt: Added.
+        * editing/selection/4402375.html: Added.
+        Turned on dumpSelectionRect for these tests:
+        * editing/selection/replaced-boundaries-1-expected.checksum:
+        * editing/selection/replaced-boundaries-1-expected.png:
+        * editing/selection/replaced-boundaries-1.html:
+        * editing/selection/replaced-boundaries-2-expected.checksum:
+        * editing/selection/replaced-boundaries-2-expected.png:
+        * editing/selection/replaced-boundaries-2.html:
+        * editing/selection/replaced-boundaries-3-expected.checksum:
+        * editing/selection/replaced-boundaries-3-expected.png:
+        * editing/selection/replaced-boundaries-3.html:
+        * editing/selection/image-before-linebreak-expected.checksum: Added.
+        * editing/selection/image-before-linebreak-expected.png: Added.
+        * editing/selection/image-before-linebreak-expected.txt: Added.
+        * editing/selection/image-before-linebreak.html: Added.
+
 2006-03-29  Adele Peterson  <adele@apple.com>
 
         Justin pointed out that this test should use an old-style form element,
diff --git a/LayoutTests/editing/selection/4402375-expected.checksum b/LayoutTests/editing/selection/4402375-expected.checksum
new file mode 100644 (file)
index 0000000..f1ecc91
--- /dev/null
@@ -0,0 +1 @@
+d7e7cdc569e376bc594c4266f480cd33
\ No newline at end of file
diff --git a/LayoutTests/editing/selection/4402375-expected.png b/LayoutTests/editing/selection/4402375-expected.png
new file mode 100644 (file)
index 0000000..af6fd3c
Binary files /dev/null and b/LayoutTests/editing/selection/4402375-expected.png differ
diff --git a/LayoutTests/editing/selection/4402375-expected.txt b/LayoutTests/editing/selection/4402375-expected.txt
new file mode 100644 (file)
index 0000000..f63aecc
--- /dev/null
@@ -0,0 +1,27 @@
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+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 {P} at (0,0) size 784x36
+        RenderText {TEXT} at (0,0) size 131x18
+          text run at (0,0) width 131: "This is a testcase for "
+        RenderInline {A} at (0,0) size 149x18 [color=#0000EE]
+          RenderText {TEXT} at (131,0) size 149x18
+            text run at (131,0) width 149: "rdar://problem/4402375"
+        RenderText {TEXT} at (280,0) size 761x36
+          text run at (280,0) width 481: " \"REGRESSION (417.8-TOT): finding text sometimes also selects previous"
+          text run at (0,18) width 137: "image (5127) (6451)\""
+      RenderBlock {HR} at (0,52) size 784x2 [border: (1px inset #000000)]
+      RenderBlock {P} at (0,70) size 784x36
+        RenderText {TEXT} at (0,0) size 743x36
+          text run at (0,0) width 413: "This test uses a right aligned image next to some left aligned text. "
+          text run at (413,0) width 330: "The image should not be selected, and should not be"
+          text run at (0,18) width 598: "included in the selection rect (you won't see the selection rect when you run this test manually)."
+      RenderBlock {DIV} at (0,122) size 784x18
+        RenderImage {IMG} at (708,0) size 76x103
+        RenderText {TEXT} at (0,0) size 178x18
+          text run at (0,0) width 178: "This text should be selected."
+selection start: position 0 of child 1 {TEXT} of child 6 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection end:   position 29 of child 1 {TEXT} of child 6 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/selection/4402375.html b/LayoutTests/editing/selection/4402375.html
new file mode 100644 (file)
index 0000000..335f5f2
--- /dev/null
@@ -0,0 +1,12 @@
+<p>This is a testcase for <a href="rdar://problem/4402375">rdar://problem/4402375</a> "REGRESSION (417.8-TOT): finding text sometimes also selects previous image (5127) (6451)"</p>
+<hr>
+<p>This test uses a right aligned image next to some left aligned text.  The image should not be selected, and should not be included in the selection rect (you won't see the selection rect when you run this test manually).</p>
+<div id="test" align="left"><img src="../resources/abe.jpg" align="right">This text should be selected.</div>
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpSelectionRect();
+var e = document.getElementById("test");
+var s = window.getSelection();
+// Select the text node.
+s.setBaseAndExtent(e, 1, e, 2);
+</script>
\ No newline at end of file
diff --git a/LayoutTests/editing/selection/image-before-linebreak-expected.checksum b/LayoutTests/editing/selection/image-before-linebreak-expected.checksum
new file mode 100644 (file)
index 0000000..46c6275
--- /dev/null
@@ -0,0 +1 @@
+65c92f4f7cc6bb9badacdadc8328d9e9
\ No newline at end of file
diff --git a/LayoutTests/editing/selection/image-before-linebreak-expected.png b/LayoutTests/editing/selection/image-before-linebreak-expected.png
new file mode 100644 (file)
index 0000000..52f07ad
Binary files /dev/null and b/LayoutTests/editing/selection/image-before-linebreak-expected.png differ
diff --git a/LayoutTests/editing/selection/image-before-linebreak-expected.txt b/LayoutTests/editing/selection/image-before-linebreak-expected.txt
new file mode 100644 (file)
index 0000000..d9707f7
--- /dev/null
@@ -0,0 +1,19 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 3 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+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 {P} at (0,0) size 784x18
+        RenderText {TEXT} at (0,0) size 341x18
+          text run at (0,0) width 341: "Only the line break after the image should be selected."
+      RenderBlock {HR} at (0,34) size 784x2 [border: (1px inset #000000)]
+      RenderBlock {DIV} at (0,44) size 784x121
+        RenderImage {IMG} at (0,0) size 76x103
+        RenderBR {BR} at (0,0) size 0x0
+        RenderText {TEXT} at (0,103) size 21x18
+          text run at (0,103) width 21: "foo"
+selection start: position 0 of child 1 {BR} of child 4 {DIV} of child 0 {BODY} of child 0 {HTML} of document
+selection end:   position 1 of child 1 {BR} of child 4 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/selection/image-before-linebreak.html b/LayoutTests/editing/selection/image-before-linebreak.html
new file mode 100644 (file)
index 0000000..afcb2f4
--- /dev/null
@@ -0,0 +1,13 @@
+<p>Only the line break after the image should be selected.</p>
+<hr>
+<div contenteditable="true" id="div"><img src="../resources/abe.jpg"><br>foo</div>
+
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpSelectionRect();
+    
+var e = document.getElementById("div");
+var s = window.getSelection();
+// Select the line break
+s.setBaseAndExtent(e, 1, e, 2);
+</script>
\ No newline at end of file
index 61f2ff5b29024c81dbc48fa818e86442e27246d1..03f304195ba9ea57f0b9adb1ce4e1797830c7d6e 100644 (file)
@@ -1 +1 @@
-47d2a1106f9f93dd18d2b5488cd44394
\ No newline at end of file
+b5393f8633c1768eb4c9b4d316771a64
\ No newline at end of file
index f9f900e7f6b05af2ce296b205cff152d3bf8b6fd..986b3e3b5786ea78b47e0ed1c5fc94dfae2bcf57 100644 (file)
Binary files a/LayoutTests/editing/selection/replaced-boundaries-1-expected.png and b/LayoutTests/editing/selection/replaced-boundaries-1-expected.png differ
index 6de843609599c355cc59fa6cfe0722638b15e685..88cc912b19738166fb71c7dd8ebed9c5508d04e5 100644 (file)
@@ -3,6 +3,8 @@
 <script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
 <script>
 function editingTest() {
+    if (window.layoutTestController)
+        window.layoutTestController.dumpSelectionRect();
     selectAllCommand();
     moveSelectionForwardByCharacterCommand();
     extendSelectionBackwardByCharacterCommand();
index d4c9834f8d0ba31b5e7b86641e593b6326edca32..dc3b9fa92971ef5836a39a65fb689e7edd98c26d 100644 (file)
@@ -1 +1 @@
-80a8c7c5e0343eb081e5d2b7289e990f
\ No newline at end of file
+9112ff78fdc868bec36aa258e8dfd04c
\ No newline at end of file
index 69ca70c5685903a19968d5c0e0fc54e718048ba5..00fb421e64f97387a9de0b4f42dbf64147f1d150 100644 (file)
Binary files a/LayoutTests/editing/selection/replaced-boundaries-2-expected.png and b/LayoutTests/editing/selection/replaced-boundaries-2-expected.png differ
index 922cc8e7390f60a8edb9d2337f04c3ae5f84557a..24839800187fa49ea267d2ab6882d39ce5c65e62 100644 (file)
@@ -3,6 +3,8 @@
 <script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
 <script>
 function editingTest() {
+    if (window.layoutTestController)
+        window.layoutTestController.dumpSelectionRect();
     selectAllCommand();
     moveSelectionBackwardByCharacterCommand();
     extendSelectionForwardByCharacterCommand();
index 85f2daf2acfc23eeb521135fb4e828f537b7dd3d..33fc48d988033548e94effda5cc5324fec3e07a2 100644 (file)
@@ -1 +1 @@
-1bf30f5b6aae392a578ac074aea5d1ab
\ No newline at end of file
+b494c34be9d0aeb1f727e17c5c63dde0
\ No newline at end of file
index 7b87060b9f33c05851520c1c53bb1b1947785eda..3be87e6d804bb3c8c8c55cd59549ccf4cf41c096 100644 (file)
Binary files a/LayoutTests/editing/selection/replaced-boundaries-3-expected.png and b/LayoutTests/editing/selection/replaced-boundaries-3-expected.png differ
index a343d9ad69ff09132bc2240e512b6f1be560e683..5efc6db88175b8155113f4cc98d21f9beba58534 100644 (file)
@@ -3,6 +3,8 @@
 <script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
 <script>
 function editingTest() {
+    if (window.layoutTestController)
+        window.layoutTestController.dumpSelectionRect();
     selectAllCommand();
     moveSelectionBackwardByCharacterCommand();
     extendSelectionForwardByLineCommand();
index ab2f14993d23c9fc92a8b9ac9ded5678cad93a4b..42630a203c30e5c8d9d294c0c6121f3649779433 100644 (file)
@@ -1,3 +1,26 @@
+2006-03-29  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+        
+        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8067>
+        REGRESSION: selectionRect includes next/previous replaced elements
+        Also fixes: <rdar://problems/4402375&4474871&4474871>
+        
+        In the case where a selection starts at the end or ends at the start
+        of o, o->selectionState() != SelectionNone, but o isn't really selected.  
+        Constraining the selection with upstream and downstream eliminates these
+        types of endpoints, but constraining endpoints that occur at the 
+        start or end of a paragraph creates positions inside containers - some 
+        of which the selection painting code isn't equipped to handle.
+
+        * dom/Document.cpp:
+        (WebCore::Document::updateSelection):
+        * rendering/render_replaced.cpp:
+        (WebCore::RenderReplaced::shouldPaint):
+        (WebCore::RenderReplaced::selectionRect):
+        (WebCore::RenderReplaced::setSelectionState):
+        (WebCore::RenderWidget::setSelectionState):
+
 2006-03-29  Adele Peterson  <adele@apple.com>
 
         Reviewed by Hyatt.
index f0b3a318b5dae7c884cd34512257d40f8f4efc7f..c800501c3b986372d496810ed8de6b610db7f2cb 100644 (file)
@@ -1046,8 +1046,10 @@ void Document::updateSelection()
     }
     
 #if __APPLE__
-    // send the AXSelectedTextChanged notification only if the new selection is non-null,
-    // because null selections are only transitory (e.g. when starting an EditCommand, currently)
+    // FIXME: We shouldn't post this AX notification here since updateSelection() is called far to often: every time Safari gains
+    // or loses focus, and once for every low level change to the selection during an editing operation.
+    // FIXME: We no longer blow away the selection before starting an editing operation, so the isNotNull checks below are no 
+    // longer a correct way to check for user-level selection changes.
     if (AccessibilityObjectCache::accessibilityEnabled() && s.start().isNotNull() && s.end().isNotNull()) {
         getAccObjectCache()->postNotificationToTopWebArea(renderer(), "AXSelectedTextChanged");
     }
index b9dd46fb40952fc847fa3d7ed59dda698a27a319..e66d9a8e5cb876e0f4bd101e29f19664fa8006d5 100644 (file)
@@ -70,7 +70,7 @@ bool RenderReplaced::shouldPaint(PaintInfo& i, int& _tx, int& _ty)
     // Early exit if the element touches the edges.
     int top = ty;
     int bottom = ty + m_height;
-    if (m_selectionState != SelectionNone && m_inlineBoxWrapper) {
+    if (isSelected() && m_inlineBoxWrapper) {
         int selTop = _ty + m_inlineBoxWrapper->root()->selectionTop();
         int selBottom = _ty + selTop + m_inlineBoxWrapper->root()->selectionHeight();
         top = kMin(selTop, top);
@@ -165,7 +165,7 @@ VisiblePosition RenderReplaced::positionForCoordinates(int _x, int _y)
 
 IntRect RenderReplaced::selectionRect()
 {
-    if (selectionState() == SelectionNone)
+    if (!isSelected())
         return IntRect();
     if (!m_inlineBoxWrapper)
         // We're a block-level replaced element.  Just return our own dimensions.
@@ -195,7 +195,7 @@ void RenderReplaced::setSelectionState(SelectionState s)
     if (m_inlineBoxWrapper) {
         RootInlineBox* line = m_inlineBoxWrapper->root();
         if (line)
-            line->setHasSelectedChildren(s != SelectionNone);
+            line->setHasSelectedChildren(isSelected());
     }
     
     containingBlock()->setSelectionState(s);
@@ -466,7 +466,7 @@ void RenderWidget::setSelectionState(SelectionState s)
         RenderReplaced::setSelectionState(s);
         m_selectionState = s;
         if (m_widget)
-            m_widget->setIsSelected(m_selectionState != SelectionNone);
+            m_widget->setIsSelected(isSelected());
     }
 }
 
index 910e11276c2068827f87951dad21db4f6bab9bfb..f51dc0b834008e9c2c0d65cba5118edaa95c106b 100644 (file)
@@ -1,3 +1,18 @@
+2006-03-28  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        <rdar://problem/4402375>
+        REGRESSION (417.8-TOT): selectionRect sometimes includes adjacent images
+
+        Added an option to draw the selectionRect.
+
+        * DumpRenderTree/DumpRenderTree.m:
+        (dump):
+        (+[LayoutTestController isSelectorExcludedFromWebScript:]):
+        (-[LayoutTestController dumpSelectionRect]):
+        (dumpRenderTree):
+        
 2006-03-29  Darin Adler  <darin@apple.com>
 
         Reviewed by Tim Hatcher.
index 839aa0ce2e74e2513ecdfa321ff2f176e4664775..4d3bb4ce0cffeb91fe4a068fd63fc3be5a9a46fd 100644 (file)
@@ -38,6 +38,7 @@
 #import <WebKit/WebPreferences.h>
 #import <WebKit/WebView.h>
 #import <WebKit/WebHTMLViewPrivate.h>
+#import <WebKit/WebDocumentPrivate.h>
 #import <WebKit/WebPluginDatabase.h>
 
 #import <ApplicationServices/ApplicationServices.h> // for CMSetDefaultProfileBySpace
@@ -78,6 +79,7 @@ static NavigationController *navigationController;
 static BOOL readyToDump;
 static BOOL waitToDump;
 static BOOL dumpAsText;
+static BOOL dumpSelectionRect;
 static BOOL dumpTitleChanges;
 static int dumpPixels = NO;
 static int testRepaintDefault = NO;
@@ -385,6 +387,13 @@ static void dump(void)
                     column.origin.x++;
                 }
             }
+            if (dumpSelectionRect) {
+                NSView *documentView = [[frame frameView] documentView];
+                if ([documentView conformsToProtocol:@protocol(WebDocumentSelection)]) {
+                    [[NSColor redColor] set];
+                    [NSBezierPath strokeRect:[documentView convertRect:[(id <WebDocumentSelection>)documentView selectionRect] fromView:nil]];
+                }
+            }
             [NSGraphicsContext restoreGraphicsState];
             
             // has the actual hash to compare to the expected image's hash
@@ -512,6 +521,7 @@ static void dump(void)
             || aSelector == @selector(dumpTitleChanges)
             || aSelector == @selector(setWindowIsKey:)
             || aSelector == @selector(setMainFrameIsFirstResponder:)
+            || aSelector == @selector(dumpSelectionRect)
             || aSelector == @selector(testRepaint)
             || aSelector == @selector(repaintSweepHorizontally))
         return NO;
@@ -544,6 +554,11 @@ static void dump(void)
     dumpAsText = YES;
 }
 
+- (void)dumpSelectionRect
+{
+    dumpSelectionRect = YES;
+}
+
 - (void)dumpTitleChanges
 {
     dumpTitleChanges = YES;
@@ -609,6 +624,7 @@ static void dumpRenderTree(const char *pathOrURL)
     readyToDump = NO;
     waitToDump = NO;
     dumpAsText = NO;
+    dumpSelectionRect = NO;
     dumpTitleChanges = NO;
     testRepaint = testRepaintDefault;
     repaintSweepHorizontally = repaintSweepHorizontallyDefault;