Attachment 223078 Details for Bug 127687 – Patch for landing

[patch] Patch for landing

bug-127687-20140204100915.patch (text/plain), 10.60 KB, created by Mihnea Ovidenie on 2014-02-04 00:07:41 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mihnea Ovidenie

Created: 2014-02-04 00:07:41 PST

Size: 10.60 KB

patch

obsolete

>Subversion Revision: 163366
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 53f3349ea54ae2c58d8123b8ccd3b5afae041978..ccc008c53f13dfd4960d32863a0e4c5951a1e7b1 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2014-02-03  Mihnea Ovidenie  <mihnea@adobe.com>
>+
>+        ASSERTION FAILED: !object || (object->isRenderBlock())
>+        https://bugs.webkit.org/show_bug.cgi?id=127687
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        Currently, when computing the visible position for a point inside a region,
>+        we transform the point into flow thread coordinates and delegate the processing
>+        to the first child of the flow thread which we incorrectly assume is a block.
>+        However, we can specify flow-into also on inline elements which leads to the
>+        assertion.
>+
>+        Instead of delegating the processing to the first child when the flow thread
>+        has children, delegate the computation of the visible position to the
>+        flow thread and avoid any assumption about the nature of the flow thread
>+        first child. If the flow thread does not have any children that should be
>+        rendered by the region, let the region behave like an ordinary element
>+        with no children.
>+
>+        Tests: fast/regions/selection/position-for-point-inline-content-node.html
>+               fast/regions/selection/selection-ended-in-empty-region.html
>+
>+        * rendering/RenderRegion.cpp:
>+        (WebCore::RenderRegion::positionForPoint):
>+
> 2014-02-03  Andreas Kling  <akling@apple.com>
> 
>         Remove stray vestige from ::-webkit-distributed selector.
>diff --git a/Source/WebCore/rendering/RenderRegion.cpp b/Source/WebCore/rendering/RenderRegion.cpp
>index 93f519c572024b436ce9dbbdfe98a2027a653238..351da3ee4b19ffdff2dd95705134cecd46060ab6 100644
>--- a/Source/WebCore/rendering/RenderRegion.cpp
>+++ b/Source/WebCore/rendering/RenderRegion.cpp
>@@ -110,7 +110,7 @@ VisiblePosition RenderRegion::positionForPoint(const LayoutPoint& point)
>     if (!isValid() || !m_flowThread->firstChild()) // checking for empty region blocks.
>         return RenderBlock::positionForPoint(point);
> 
>-    return toRenderBlock(m_flowThread->firstChild())->positionForPoint(mapRegionPointIntoFlowThreadCoordinates(point));
>+    return m_flowThread->positionForPoint(mapRegionPointIntoFlowThreadCoordinates(point));
> }
> 
> LayoutUnit RenderRegion::pageLogicalWidth() const
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index cd6a1c53819c5a865cabe943bd4074803d887efc..9a21fcf26a286352dba3b205f22a746f612b600b 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,21 @@
>+2014-02-03  Mihnea Ovidenie  <mihnea@adobe.com>
>+
>+        ASSERTION FAILED: !object || (object->isRenderBlock())
>+        https://bugs.webkit.org/show_bug.cgi?id=127687
>+
>+        Reviewed by Ryosuke Niwa.
>+
>+        Added test showing that the content of an inline content node, having
>+        flow-into specified, can be properly selected without hitting the assertion.
>+        Also added another test to make sure that when the selection starts
>+        outside an empty region and ends inside the empty region, the region behaves
>+        like an ordinary element with no children.
>+
>+        * fast/regions/selection/position-for-point-inline-content-node-expected.txt: Added.
>+        * fast/regions/selection/position-for-point-inline-content-node.html: Added.
>+        * fast/regions/selection/selection-ended-in-empty-region-expected.txt: Added.
>+        * fast/regions/selection/selection-ended-in-empty-region.html: Added.
>+
> 2014-02-03  Mark Lam  <mark.lam@apple.com>
> 
>         Need regression test for the debugger modifying locals in a DFG frame.
>diff --git a/LayoutTests/fast/regions/selection/position-for-point-inline-content-node-expected.txt b/LayoutTests/fast/regions/selection/position-for-point-inline-content-node-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..eec6295c87da67b574bf49af6a6fb6915a24178d
>--- /dev/null
>+++ b/LayoutTests/fast/regions/selection/position-for-point-inline-content-node-expected.txt
>@@ -0,0 +1,4 @@
>+word1 content inside region word2
>+Test that the selection inside inline content node is processed correctly.
>+
>+To manually test, select from "word1" to "word2" and move to the right edge of the region without leaving the region. Focus node should be at the end of "word2". Selected content should go from "word1" to "word2". No assertion should fire in debug mode: PASS
>diff --git a/LayoutTests/fast/regions/selection/position-for-point-inline-content-node.html b/LayoutTests/fast/regions/selection/position-for-point-inline-content-node.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f14d98af8c85e07f11b02eb6043110b508d33cbe
>--- /dev/null
>+++ b/LayoutTests/fast/regions/selection/position-for-point-inline-content-node.html
>@@ -0,0 +1,55 @@
>+<!DOCTYPE html>
>+<html style="font: 20px/1 monospace;">
>+<head>
>+    <link rel="stylesheet" href="../resources/region-style.css"></link>
>+    <style>
>+        .region {
>+            margin: 50px 0px 50px 0px;
>+            border: 5px solid black;
>+            width: 500px;
>+        }
>+
>+        #debug {
>+            border: 1px solid black;
>+        }
>+    </style>
>+    <script src="../resources/helper.js"></script>
>+    <script>
>+        onMouseUpLogSelectionAndFocus("selected-content", "focus-node", "focus-offset");
>+        function selectText() {
>+            if (window.testRunner) {
>+                window.testRunner.dumpAsText();
>+
>+                var selectedText = document.getElementById("debug");
>+                selectedText.style.display = "none";
>+
>+                var region1 = document.getElementById("region-1");
>+                var region1Rect = region1.getBoundingClientRect();
>+
>+                clearSelection();
>+
>+                selectContentFromIdToPos("source", region1Rect.right - 5, region1Rect.bottom - 5);
>+                checkSelectionResult("result", /* id of element to log the result */
>+                    "word1 content inside region word2", /* expected selection */
>+                    "source", /* expected focus node id */
>+                    "33");    /* expected focus offset */
>+            }
>+        }
>+    </script>
>+</head>
>+<body onload="selectText();">
>+    <span id="source" class="flowArticle">word1 content inside region word2</span>
>+    <div id="region-1" class="regionArticle region"></div>
>+    <div id="desc" class="description">
>+        <p>Test that the selection inside inline content node is processed correctly.</p>
>+        <p>To manually test, select from "word1" to "word2" and move to the right edge of the region without leaving the region. Focus node should be at the end of "word2". Selected content should go from "word1" to "word2". No assertion should fire in debug mode: <span id="result"></span></p>
>+    </div>
>+    <div id="debug">
>+        <ul>
>+            <li>Focus node: <span id="focus-node"></span></li>
>+            <li>Focus offset: <span id="focus-offset"></span></li>
>+            <li>Selected content: <span id="selected-content"></span></li>
>+        </ul>
>+    </div>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/regions/selection/selection-ended-in-empty-region-expected.txt b/LayoutTests/fast/regions/selection/selection-ended-in-empty-region-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..c6cc9561189cc7b106f290d61ae67e7be439fcc5
>--- /dev/null
>+++ b/LayoutTests/fast/regions/selection/selection-ended-in-empty-region-expected.txt
>@@ -0,0 +1,9 @@
>+word1 before empty region word2
>+after empty region
>+Test that when you start the selection outside an empty region and end the selection inside the empty region, only the content before the empty region is selected.
>+
>+To manually test, start selection from "word1" and move to the middle of the box with black border, representing the empty region.
>+
>+Focus node should be the empty region.
>+
>+Selected content should go from "word1" to "word2": PASS
>diff --git a/LayoutTests/fast/regions/selection/selection-ended-in-empty-region.html b/LayoutTests/fast/regions/selection/selection-ended-in-empty-region.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..c84a666c2cc9635ef3a90c6937c6645399f523dd
>--- /dev/null
>+++ b/LayoutTests/fast/regions/selection/selection-ended-in-empty-region.html
>@@ -0,0 +1,58 @@
>+<!DOCTYPE html>
>+<html style="font: 20px/1 monospace;">
>+<head>
>+    <link rel="stylesheet" href="../resources/region-style.css"></link>
>+    <style>
>+        .region {
>+            margin: 50px 0px 50px 0px;
>+            border: 5px solid black;
>+            width: 500px;
>+        }
>+
>+        #debug {
>+            border: 1px solid black;
>+        }
>+    </style>
>+    <script src="../resources/helper.js"></script>
>+    <script>
>+        onMouseUpLogSelectionAndFocus("selected-content", "focus-node", "focus-offset");
>+        function selectText() {
>+            if (window.testRunner) {
>+                window.testRunner.dumpAsText();
>+
>+                var selectedText = document.getElementById("debug");
>+                selectedText.style.display = "none";
>+
>+                var region1 = document.getElementById("region-1");
>+                var region1Rect = region1.getBoundingClientRect();
>+
>+                clearSelection();
>+
>+                selectContentByIds("beforeRegion", "region-1");
>+                checkSelectionResult("result", /* id of element to log the result */
>+                    "word1 before empty region word2", /* expected selection */
>+                    "region-1", /* expected focus node id */
>+                    "0");    /* expected focus offset */
>+            }
>+        }
>+    </script>
>+</head>
>+<body onload="selectText();">
>+    <div id="beforeRegion">word1 before empty region word2</div>
>+    <div id="region-1" class="greyBigBox regionArticle region"></div>
>+    <div id="afterRegion">after empty region</div>
>+    <div id="desc" class="description">
>+        <p>Test that when you start the selection outside an empty region and end the selection inside the empty region, only the content before the empty region is selected.</p>
>+        <p>To manually test, start selection from "word1" and move to the middle of the box with black border, representing the empty region.</p>
>+        <p>Focus node should be the empty region.</p>
>+        <p>Selected content should go from "word1" to "word2": <span id="result"></span></p>
>+    </div>
>+    <div id="debug">
>+        <ul>
>+            <li>Focus node: <span id="focus-node"></span></li>
>+            <li>Focus offset: <span id="focus-offset"></span></li>
>+            <li>Selected content: <span id="selected-content"></span></li>
>+        </ul>
>+    </div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 127687: 222336 | 222558 | 222667 | 223078