<https://webkit.org/b/119622> [CSSRegions] Not possible to clear the selection when...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Sep 2013 19:52:25 +0000 (19:52 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Sep 2013 19:52:25 +0000 (19:52 +0000)
Patch by Javier Fernandez <jfernandez@igalia.com> on 2013-09-04
Reviewed by David Hyatt.

Source/WebCore:

When using CSS Regions is usual that the RenderTree doesn't match
the DOM Tree in terms of relative position of the nodes. Besides,
usually the content of a certain node is split and spread across
multiple blocks, rendered in different positions.

Regarding the Selection, this problem is even more important; the
selection direction changes when crossing the FlowThread
boundaries. This weird behavior is also present in other layouts
using non-regular positioning mechanisms, like absolute,
static. However, for those layouts the RenderTree preserves the
order of the nodes, unlike the CSS Regions layout model.

Because of how the RenderTree is generated with CSS Regions, the
RenderView::setSelection algorithm is not able to repaint some of
the rectangles defined during the selection process. In order to
face this issue, the proposed fix determines whether it should
backwards traversing the RenderTree, from the "stop" node to the
RenderView node.

Test: fast/regions/selecting-text-through-different-region-flows-2.html

* rendering/RenderView.cpp:
(WebCore::getNextOrPrevRenderObjectBasedOnDirection): Added.
(WebCore::RenderView::setSelection):

LayoutTests:

* fast/regions/selecting-text-through-different-region-flows-2-expected.html: Added.
* fast/regions/selecting-text-through-different-region-flows-2.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/regions/selecting-text-through-different-region-flows-2-expected.html [new file with mode: 0644]
LayoutTests/fast/regions/selecting-text-through-different-region-flows-2.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderView.cpp

index 3b86abef6eac68446376c981117d64ae1945498e..87ce900ccd8adeb6548c6cb98ab2e4b1fc731d45 100644 (file)
@@ -1,3 +1,12 @@
+2013-09-04  Javier Fernandez  <jfernandez@igalia.com>
+
+        <https://webkit.org/b/119622> [CSSRegions] Not possible to clear the selection when mixing content from different FlowThreads
+
+        Reviewed by David Hyatt.
+
+        * fast/regions/selecting-text-through-different-region-flows-2-expected.html: Added.
+        * fast/regions/selecting-text-through-different-region-flows-2.html: Added.
+
 2013-09-04  Oliver Hunt  <oliver@apple.com>
 
         Fix fast/storage/serialized script value, and add new tests for Map and Set
diff --git a/LayoutTests/fast/regions/selecting-text-through-different-region-flows-2-expected.html b/LayoutTests/fast/regions/selecting-text-through-different-region-flows-2-expected.html
new file mode 100644 (file)
index 0000000..b2aeb5e
--- /dev/null
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body { width: 600px; }
+#footNote { font-size: 13px; }
+</style>
+<script>
+function selectText()
+{
+  var selection = window.getSelection();
+
+  var start = document.getElementById("start");
+  var end = document.getElementById("end");
+
+  selection.setBaseAndExtent(start, 0, end, 0);
+}
+</script>
+</head>
+<body onload="selectText()">
+<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>
+</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/regions/selecting-text-through-different-region-flows-2.html b/LayoutTests/fast/regions/selecting-text-through-different-region-flows-2.html
new file mode 100644 (file)
index 0000000..e283e02
--- /dev/null
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body { width: 600px; }
+
+#footNote {
+    -webkit-flow-into: footNote;
+    display: block;
+    font-size: 13px;
+}
+
+#footNoteRegion { -webkit-flow-from: footNote; }
+#content { -webkit-flow-into: content; }
+#region { -webkit-flow-from: content; }
+</style>
+</head>
+<body>
+
+<div id="region"></div>
+<div id="footNoteRegion"></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>
+</div>
+
+<script>
+if (window.testRunner) {
+    // We are positioning the mouse to the center of the contentText and start holding the mouse down
+    var start = document.getElementById("start");
+    var xStartPosition = start.offsetLeft + 0;
+    var yStartPosition = start.offsetTop + start.offsetHeight / 2;
+    eventSender.mouseMoveTo(xStartPosition, yStartPosition);
+    eventSender.mouseDown();
+
+    // We are positioning the mouse to the center of the footNote (what is a different region flow) and release the button
+    var end = document.getElementById("end");
+    var xEndPosition = end.offsetLeft + 0;
+    var yEndPosition = end.offsetTop + end.offsetHeight / 2;
+    eventSender.mouseMoveTo(xEndPosition, yEndPosition);
+    eventSender.mouseUp();
+}
+</script>
+</body>
+</html>
index d30ca07f306af0974e7baa489facd3531972f1c5..d9b9ff9b5d74bbd2493454e00f5bb5f4935dfe9b 100644 (file)
@@ -1,3 +1,34 @@
+2013-09-04  Javier Fernandez  <jfernandez@igalia.com>
+
+        <https://webkit.org/b/119622> [CSSRegions] Not possible to clear the selection when mixing content from different FlowThreads
+
+        Reviewed by David Hyatt.
+
+        When using CSS Regions is usual that the RenderTree doesn't match
+        the DOM Tree in terms of relative position of the nodes. Besides,
+        usually the content of a certain node is split and spread across
+        multiple blocks, rendered in different positions.
+
+        Regarding the Selection, this problem is even more important; the
+        selection direction changes when crossing the FlowThread
+        boundaries. This weird behavior is also present in other layouts
+        using non-regular positioning mechanisms, like absolute,
+        static. However, for those layouts the RenderTree preserves the
+        order of the nodes, unlike the CSS Regions layout model.
+
+        Because of how the RenderTree is generated with CSS Regions, the
+        RenderView::setSelection algorithm is not able to repaint some of
+        the rectangles defined during the selection process. In order to
+        face this issue, the proposed fix determines whether it should
+        backwards traversing the RenderTree, from the "stop" node to the
+        RenderView node.
+
+        Test: fast/regions/selecting-text-through-different-region-flows-2.html
+
+        * rendering/RenderView.cpp:
+        (WebCore::getNextOrPrevRenderObjectBasedOnDirection): Added.
+        (WebCore::RenderView::setSelection):
+
 2013-09-04  Eric Carlson  <eric.carlson@apple.com>
 
         Get MEDIA_STREAM compiling on OSX
index 8a61d1ae74f215c2989612d03a3db02338356c6b..692f23fa7cabf8cfda393f5759514c4ef9100b4f 100644 (file)
@@ -736,6 +736,27 @@ void RenderView::setMaximalOutlineSize(int o)
 }
 #endif
 
+// When exploring the RenderTree looking for the nodes involved in the Selection, sometimes it's
+// required to change the traversing direction because the "start" position is below the "end" one.
+static inline RenderObject* getNextOrPrevRenderObjectBasedOnDirection(const RenderObject* o, const RenderObject* stop, bool& continueExploring, bool& exploringBackwards)
+{
+    RenderObject* next;
+    if (exploringBackwards) {
+        next = o->previousInPreOrder();
+        continueExploring = next && !(next)->isRenderView();
+    } else {
+        next = o->nextInPreOrder();
+        continueExploring = next && next != stop;
+        exploringBackwards = !next && (next != stop);
+        if (exploringBackwards) {
+            next = stop->previousInPreOrder();
+            continueExploring = next && !next->isRenderView();
+        }
+    }
+
+    return next;
+}
+
 void RenderView::setSelection(RenderObject* start, int startPos, RenderObject* end, int endPos, SelectionRepaintMode blockRepaintMode)
 {
     // Make sure both our start and end objects are defined.
@@ -750,9 +771,6 @@ void RenderView::setSelection(RenderObject* start, int startPos, RenderObject* e
         m_selectionEnd == end && m_selectionEndPos == endPos && !caretChanged)
         return;
 
-    if ((start && end) && (start->flowThreadContainingBlock() != end->flowThreadContainingBlock()))
-        return;
-
     // Record the old selected objects.  These will be used later
     // when we compare against the new selected objects.
     int oldStartPos = m_selectionStartPos;
@@ -772,7 +790,9 @@ void RenderView::setSelection(RenderObject* start, int startPos, RenderObject* e
 
     RenderObject* os = m_selectionStart;
     RenderObject* stop = rendererAfterPosition(m_selectionEnd, m_selectionEndPos);
-    while (os && os != stop) {
+    bool exploringBackwards = false;
+    bool continueExploring = os && (os != stop);
+    while (continueExploring) {
         if ((os->canBeSelectionLeaf() || os == m_selectionStart || os == m_selectionEnd) && os->selectionState() != SelectionNone) {
             // Blocks are responsible for painting line gaps and margin gaps.  They must be examined as well.
             oldSelectedObjects.set(os, adoptPtr(new RenderSelectionInfo(os, true)));
@@ -788,7 +808,7 @@ void RenderView::setSelection(RenderObject* start, int startPos, RenderObject* e
             }
         }
 
-        os = os->nextInPreOrder();
+        os = getNextOrPrevRenderObjectBasedOnDirection(os, stop, continueExploring, exploringBackwards);
     }
 
     // Now clear the selection.
@@ -827,7 +847,9 @@ void RenderView::setSelection(RenderObject* start, int startPos, RenderObject* e
     // Now that the selection state has been updated for the new objects, walk them again and
     // put them in the new objects list.
     o = start;
-    while (o && o != stop) {
+    exploringBackwards = false;
+    continueExploring = o && (o != stop);
+    while (continueExploring) {
         if ((o->canBeSelectionLeaf() || o == start || o == end) && o->selectionState() != SelectionNone) {
             newSelectedObjects.set(o, adoptPtr(new RenderSelectionInfo(o, true)));
             RenderBlock* cb = o->containingBlock();
@@ -840,7 +862,7 @@ void RenderView::setSelection(RenderObject* start, int startPos, RenderObject* e
             }
         }
 
-        o = o->nextInPreOrder();
+        o = getNextOrPrevRenderObjectBasedOnDirection(o, stop, continueExploring, exploringBackwards);
     }
 
     if (blockRepaintMode == RepaintNothing)