[CSS Regions] Selection highlight doesn't match DOM selection

https://bugs.webkit.org/show_bug.cgi?id=131511 Reviewed by David Hyatt. Source/WebCore: This patch implements a subtrees approach to fix selection interaction in CSS Regions. The idea is to divide the Render tree in subtrees. One subtree will be the RenderView (which is always present), then for each RenderNamedFlowThread (which are children of RenderView) we will have an extra subtree. For each subtree it determines the start and end positions according to the DOM tree. So, every subtree will always have a start and end position, and the selection algorithm will be able to reach the end from the start without problems (this allows us to remove the changes introduced in r155058). Then selection related methods in RenderView are adapted to ensure that they are called for each subtree. Making the selection highlight to match DOM selection as expected. Test: fast/regions/selection/selecting-text-including-region.html * CMakeLists.txt: Add new class SelectionSubtreeRoot to build file. * WebCore.vcxproj/WebCore.vcxproj: Ditto. * WebCore.vcxproj/WebCore.vcxproj.filters: Ditto. * WebCore.xcodeproj/project.pbxproj: Ditto. * rendering/RenderNamedFlowThread.h: Inherit from SelectionSubtreeRoot. * rendering/RenderObject.cpp: (WebCore::RenderObject::selectionRoot): New method to return the RenderNamedFlowThread if any or the RenderView otherwise. (WebCore::RenderObject::selectionStartEnd): If it's inside a RenderNamedFlowThread use specific method instead of the RenderView's one. * rendering/RenderObject.h: Add new method signature. * rendering/RenderView.cpp: (WebCore::RenderView::selectionBounds): Adapt method to subtrees approach. (WebCore::RenderView::subtreeSelectionBounds): Modify previous method in order to use SelectionSubtreeRoot objects. (WebCore::RenderView::repaintSelection): Adapt method to subtrees approach. (WebCore::RenderView::repaintSubtreeSelection): Modify previous method in order to use SelectionSubtreeRoot objects. (getNextOrPrevRenderObjectBasedOnDirection): Deleted method from r155058 as it is not needed anymore. Each subtree will have start and end positions, so the selection algorithm will always find the end. (WebCore::RenderView::setSelection): Adapt method to subtrees approach. (WebCore::RenderView::splitSelectionBetweenSubtrees): Determine start and end positions for each subtree and call setSubtreeSelection(). (WebCore::RenderView::setSubtreeSelection): Modify previous method in order to use SelectionSubtreeRoot objects. * rendering/RenderView.h: Inherit from SelectionSubtreeRoot. Add signature for new helper methods receiving SelectionSubtreeRoot objects. * rendering/SelectionSubtreeRoot.cpp: Added. (WebCore::SelectionSubtreeRoot::SelectionSubtreeRoot): (WebCore::SelectionSubtreeRoot::selectionStartEndPositions): * rendering/SelectionSubtreeRoot.h: Added. New class to identify root elements for the subtrees. With some basic attributes and methods to store the start and end positions and offsets. (WebCore::SelectionSubtreeRoot::selectionStart): (WebCore::SelectionSubtreeRoot::selectionStartPos): (WebCore::SelectionSubtreeRoot::selectionEnd): (WebCore::SelectionSubtreeRoot::selectionEndPos): (WebCore::SelectionSubtreeRoot::setSelectionStart): (WebCore::SelectionSubtreeRoot::setSelectionStartPos): (WebCore::SelectionSubtreeRoot::setSelectionEnd): (WebCore::SelectionSubtreeRoot::setSelectionEndPos): LayoutTests: Add new test to check highlight of render objects moved under RenderNamedFlowThread in the render tree. Updated some tests to avoid issues with selection gap filling in the new implementation. * fast/regions/selection/selecting-text-including-region-expected.html: Added. * fast/regions/selection/selecting-text-including-region.html: Added. * fast/regions/selection/selecting-text-through-different-region-flows-2-expected.html: Updated. * fast/regions/selection/selecting-text-through-different-region-flows-2.html: Updated. * fast/regions/selection/selection-direction-expected.html: Updated. * fast/regions/selection/selection-direction.html: Updated. Canonical link: https://commits.webkit.org/150070@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@167652 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Apr 22, 2014 · 8399ed3b3e34cab9265c34036726d4031d3f9c77 · 8399ed3
1 parent bbeec59
commit 8399ed3b3e34cab9265c34036726d4031d3f9c77
Show file tree

Hide file tree

Showing 19 changed files with 666 additions and 92 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,21 @@
+2014-04-22  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [CSS Regions] Selection highlight doesn't match DOM selection
+        https://bugs.webkit.org/show_bug.cgi?id=131511
+
+        Reviewed by David Hyatt.
+
+        Add new test to check highlight of render objects moved under RenderNamedFlowThread in the render tree.
+
+        Updated some tests to avoid issues with selection gap filling in the new implementation.
+
+        * fast/regions/selection/selecting-text-including-region-expected.html: Added.
+        * fast/regions/selection/selecting-text-including-region.html: Added.
+        * fast/regions/selection/selecting-text-through-different-region-flows-2-expected.html: Updated.
+        * fast/regions/selection/selecting-text-through-different-region-flows-2.html: Updated.
+        * fast/regions/selection/selection-direction-expected.html: Updated.
+        * fast/regions/selection/selection-direction.html: Updated.
+
 2014-04-21  Brent Fulgham  <bfulgham@apple.com>
 
         [Mac] After a horizontal overflow scroll with a mechanical wheel mouse, vertical scrolling no longer works

diff --git a/LayoutTests/fast/regions/selection/selecting-text-including-region-expected.html b/LayoutTests/fast/regions/selection/selecting-text-including-region-expected.html
@@ -0,0 +1,74 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="../resources/helper.js"></script>
+    <link rel="stylesheet" href="../resources/region-style.css"></link>
+    <style>
+        * {
+            margin: 0px;
+            padding: 0px;
+        }
+
+        body {
+            line-height: 20px;
+            font-size: 20px;
+            font-family: Ahem;
+        }
+
+        #outside-1 {
+            position: absolute;
+            top: 0px;
+        }
+
+        #outside-2 {
+            position: absolute;
+            top: 100px;
+        }
+
+        #region {
+            position: absolute;
+            top: 200px;
+            background-color: cyan;
+        }
+
+        #description {
+            position: absolute;
+            top: 350px;
+        }
+    </style>
+    <script>
+        onMouseUpLogSelection("selected-content");
+    </script>
+</head>
+<body onload="selectContentByIds('word1', 'word2');" style="margin-top: 0;">
+    <div id="outside-1" class="bigBox">
+        X X X X X X X X
+        X X X X X X X X
+        X X X <span id="word1" class="token">X</span> X X X X
+        X X X X X X X X
+        X X X X X X X X
+    </div>
+    <div id="region" class="bigBox">
+        X X X X X X X X
+        X X X X X X X X
+        X X X X X X X X
+        X X X X X X X X
+        X X X X X X X X
+    </div>
+    <div id="outside-2" class="bigBox">
+        X X X X X X X X
+        X X X X X X X X
+        X X X <span id="word2" class="token">X</span> X X X X
+        X X X X X X X X
+        X X X X X X X X
+    </div>
+    <div id="description" class="description">
+        <h1><a href="https://bugs.webkit.org/show_bug.cgi?id=131511">Bug #131511 - [CSS Regions] Selection highlight doesn't match DOM selection</a></h1>
+        <p>This test checks the selection in 2 text wrapping a content node. If you select from <span class="token">X</span> to <span class="token">X</span> you will see that region text is highlighted and included in the selected content (because of the content of the region is between bot texts in the DOM).</p>
+        <dl>
+            <dt>Selected content:</dt>
+            <dd id="selected-content"></dd>
+        </dl>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/fast/regions/selection/selecting-text-including-region.html b/LayoutTests/fast/regions/selection/selecting-text-including-region.html
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="../resources/helper.js"></script>
+    <link rel="stylesheet" href="../resources/region-style.css"></link>
+    <style>
+        * {
+            margin: 0px;
+            padding: 0px;
+        }
+
+        body {
+            line-height: 20px;
+            font-size: 20px;
+            font-family: Ahem;
+        }
+
+        #region {
+            background-color: cyan;
+        }
+
+        #description {
+            position: absolute;
+            top: 350px;
+        }
+    </style>
+    <script>
+        onMouseUpLogSelection("selected-content");
+    </script>
+</head>
+<body onload="selectContentByIds('word1', 'word2');" style="margin-top: 0;">
+    <div class="bigBox float">
+        X X X X X X X X
+        X X X X X X X X
+        X X X <span id="word1" class="token">X</span> X X X X
+        X X X X X X X X
+        X X X X X X X X
+    </div>
+    <div class="clear"></div>
+    <div class="flowArticle">
+        X X X X X X X X
+        X X X X X X X X
+        X X X X X X X X
+        X X X X X X X X
+        X X X X X X X X
+    </div>
+    <div class="bigBox float">
+        X X X X X X X X
+        X X X X X X X X
+        X X X <span id="word2" class="token">X</span> X X X X
+        X X X X X X X X
+        X X X X X X X X
+    </div>
+    <div class="clear"></div>
+    <div id="region" class="regionArticle bigBox"></div>
+    <div id="description" class="description">
+        <h1><a href="https://bugs.webkit.org/show_bug.cgi?id=131511">Bug #131511 - [CSS Regions] Selection highlight doesn't match DOM selection</a></h1>
+        <p>This test checks the selection in 2 text wrapping a content node. If you select from <span class="token">X</span> to <span class="token">X</span> you will see that region text is highlighted and included in the selected content (because of the content of the region is between bot texts in the DOM).</p>
+        <dl>
+            <dt>Selected content:</dt>
+            <dd id="selected-content"></dd>
+        </dl>
+    </div>
+</body>
+</html>
diff --git a/...ests/fast/regions/selection/selecting-text-through-different-region-flows-2-expected.html b/...ests/fast/regions/selection/selecting-text-through-different-region-flows-2-expected.html
@@ -3,19 +3,56 @@
 <head>
 <script src="../resources/helper.js"></script>
 <style>
-body { width: 600px; }
-#footNote { font-size: 13px; }
+* {
+    margin: 0px;
+    padding: 0px;
+    font-size: 20px;
+    font-family: Ahem;
+    line-height: 20px;
+}
+#content1 {
+    height: 100px;
+    width: 200px;
+    position: absolute;
+    top: 50px;
+}
+#nestedRegion {
+    height: 100px;
+    width: 200px;
+    color: red;
+    position: absolute;
+    top: 250px;
+}
+#content2 {
+    height: 100px;
+    width: 200px;
+    position: absolute;
+    top: 150px;
+}
 </style>
 </head>
 <body onload="selectBaseAndExtent('start', 0, 'end', 1)">
-<div id="content">
-    <h1 style="margin-top: 0px">Selecting text through different CSS-Region flows</h1>
-    <div>
-    This text contains a footnote as a nested region what is diplayed <span id="start">below<span> the article. (1) If you start selecting
-        text from this article until somewhere in the footnote and then click somewhere, the selection should be cleared.
-    </div>
-    <div id="footNote">(1) This is a footnote. Footnotes can be quite long and go over several lines. This footnote
-    is nested inside the text <span id="end">above<span> and displayed here with the help of css-regions.</span></div>
+<h1>Selecting text through different CSS-Region flows</h1>
+<div id="content1">
+    X X X X X
+    X X X X X
+    X X <span id="start">X</span> X X
+    X X X X X
+    X X X X XX
+</div>
+<div id="nestedRegion">
+    X X X X X
+    X X X X X
+    X X <span id="end">X</span> X X
+    X X X X X
+    X X X X X
+</div>
+<div id="content2">
+    X X X X X
+    X X X X X
+    X X X X X
+    X X X X X
+    X X X X X
 </div>
 </body>
 </html>
diff --git a/LayoutTests/fast/regions/selection/selecting-text-through-different-region-flows-2.html b/LayoutTests/fast/regions/selection/selecting-text-through-different-region-flows-2.html
@@ -3,30 +3,73 @@
 <head>
 <script src="../resources/helper.js"></script>
 <style>
-body { width: 600px; }
+* {
+    margin: 0px;
+    padding: 0px;
+    font-size: 20px;
+    font-family: Ahem;
+    line-height: 20px;
+}
+
+#nestedRegion {
+    -webkit-flow-into: nestedRegion;
+    color: red;
+}
 
-#footNote {
-    -webkit-flow-into: footNote;
-    display: block;
-    font-size: 13px;
+#content {
+    -webkit-flow-into: content;
 }
 
-#footNoteRegion { -webkit-flow-from: footNote; }
-#content { -webkit-flow-into: content; }
-#region { -webkit-flow-from: content; }
+#nestedRegionRegion {
+    -webkit-flow-from: nestedRegion;
+    height: 100px;
+    width: 200px;
+}
+#region {
+    -webkit-flow-from: content;
+    height: 200px;
+    width: 200px;
+}
+
+#content1 {
+    height: 100px;
+}
+
+#content2 {
+    height: 100px;
+}
+
+h1 {
+    height: 50px;
+}
 </style>
 </head>
 <body>
 
+<h1>Selecting text through different CSS-Region flows</h1>
 <div id="region"></div>
-<div id="footNoteRegion"></div>
+<div id="nestedRegionRegion"></div>
 <div id="content">
-    <h1 style="margin-top: 0px">Selecting text through different CSS-Region flows</h1>
-    <div id="contentText">
-        This text contains a footnote as a nested region what is diplayed <span id="start">below</span> the article. (1) <span id="footNote">
-        <span><span></span>(1) This is a footnote. Footnotes can be quite long and go over several lines. This footnote
-        is nested inside the text <span id="end">above</span> and displayed here with the help of css-regions.</span></span> If you start selecting
-        text from this article until somewhere in the footnote and then click somewhere, the selection should be cleared.
+    <div id="content1">
+        X X X X X
+        X X X X X
+        X X <span id="start">X</span> X X
+        X X X X X
+        X X X X XX
+    </div>
+    <div id="nestedRegion">
+        X X X X X
+        X X X X X
+        X X <span id="end">X</span> X X
+        X X X X X
+        X X X X X
+    </div>
+    <div id="content2">
+        X X X X X
+        X X X X X
+        X X X X X
+        X X X X X
+        X X X X X
     </div>
 </div>
 

diff --git a/LayoutTests/fast/regions/selection/selection-direction-expected.html b/LayoutTests/fast/regions/selection/selection-direction-expected.html
@@ -7,32 +7,53 @@
             width: 200px;
             position: absolute;
             top: 100px;
+            color: red;
         }
 
         #outside {
             height: 100px;
             width: 200px;
             position: absolute;
             top: 0px;
+            color: green;
         }
 
         body {
             margin-top: 0;
+            font-size: 20px;
+            font-family: Ahem;
+            line-height: 20px;
         }
 
         #description {
             width: 600px;
             position: absolute;
             top: 200px;
         }
+
+        h1 {
+            line-height: 50px;
+        }
     </style>
 </head>
-<body onload="selectContentByRange(100, 10, 100, 110);">
-    <div id="region">inside region inside region inside region inside region</div>
-    <div id="outside">outside region outside region outside region outside region</div>
+<body onload="selectContentByRange(100, 50, 100, 150);">
+    <div id="region">
+        X X X X X
+        X X X X X
+        X X X X X
+        X X X X X
+        X X X X XX
+    </div>
+    <div id="outside">
+        X X X X X
+        X X X X X
+        X X X X X
+        X X X X X
+        X X X X X
+    </div>
     <div id="description">
         <h1><a href="https://bugs.webkit.org/show_bug.cgi?id=121501">Bug #121501 - [CSS Regions] Layout Test for selection direction</a></h1>
-        <p>Selection is based in DOM tree, so if you select text from "outside region" to "inside region" the highlighted area is exactly the opposite to what the user is actually selecting.</p>
+        <p>Selection is based in DOM tree, so if you select text from "green" to "red" the highlighted area is exactly the opposite to what the user is actually selecting.</p>
     </div>
 </body>
 </html>