Title: [282816] trunk
- Revision
- 282816
- Author
- [email protected]
- Date
- 2021-09-21 08:05:51 -0700 (Tue, 21 Sep 2021)
Log Message
REGRESSION(r282129): Double clicking margin of a block inside a <span> may select a wrong block
https://bugs.webkit.org/show_bug.cgi?id=230535
Reviewed by Alan Bujtas.
Source/WebCore:
If the blocks are inside a <span> we fail to search through continuation chain
and end up always selecting the first block inside it.
Test: editing/selection/hit-test-continuation-margin.html
* rendering/RenderInline.cpp:
(WebCore::RenderInline::positionForPoint):
The firstLineBox() test was really an "are inlines culled" test which stopped making sense
after inline culling was removed. Continuations need special handling but we would only
get to that code path if inlines were culled. After r282129 we would never get there.
Fix by removing the hasInlineBox test.
LayoutTests:
* editing/selection/hit-test-continuation-margin-expected.html: Added.
* editing/selection/hit-test-continuation-margin.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (282815 => 282816)
--- trunk/LayoutTests/ChangeLog 2021-09-21 14:54:39 UTC (rev 282815)
+++ trunk/LayoutTests/ChangeLog 2021-09-21 15:05:51 UTC (rev 282816)
@@ -1,5 +1,15 @@
2021-09-21 Antti Koivisto <[email protected]>
+ REGRESSION(r282129): Double clicking margin of a block inside a <span> may select a wrong block
+ https://bugs.webkit.org/show_bug.cgi?id=230535
+
+ Reviewed by Alan Bujtas.
+
+ * editing/selection/hit-test-continuation-margin-expected.html: Added.
+ * editing/selection/hit-test-continuation-margin.html: Added.
+
+2021-09-21 Antti Koivisto <[email protected]>
+
[LFC][Integration] Enable markers and highlights
https://bugs.webkit.org/show_bug.cgi?id=230542
Added: trunk/LayoutTests/editing/selection/hit-test-continuation-margin-expected.html (0 => 282816)
--- trunk/LayoutTests/editing/selection/hit-test-continuation-margin-expected.html (rev 0)
+++ trunk/LayoutTests/editing/selection/hit-test-continuation-margin-expected.html 2021-09-21 15:05:51 UTC (rev 282816)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#test { width: 300px; font-size: 50px; border: solid 1px black; padding: 5px; }
+#test span { color: green; }
+#test div { border: solid 2px blue; margin-top: 100px;}
+</style>
+</head>
+<body>
+<p>Double-clicking in the empty space (margin) after the second line should select the second line, not the first.</p>
+<div style="padding: 50px;">
+<div id="test"><span><div>first</div><div id="target">second</div><div>third</div></span></div>
+</div>
+<script>
+
+const target = document.querySelector('#target');
+
+var range = document.createRange();
+range.setStart(target, 0);
+range.setEnd(target, 1);
+window.getSelection().addRange(range);
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/editing/selection/hit-test-continuation-margin.html (0 => 282816)
--- trunk/LayoutTests/editing/selection/hit-test-continuation-margin.html (rev 0)
+++ trunk/LayoutTests/editing/selection/hit-test-continuation-margin.html 2021-09-21 15:05:51 UTC (rev 282816)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+#test { width: 300px; font-size: 50px; border: solid 1px black; padding: 5px; }
+#test span { color: green; }
+#test div { border: solid 2px blue; margin-top: 100px;}
+</style>
+</head>
+<body>
+<p>Double-clicking in the empty space (margin) after the second line should select the second line, not the first.</p>
+<div style="padding: 50px;">
+<div id="test"><span><div>first</div><div id="target">second</div><div>third</div></span></div>
+</div>
+<script>
+
+const target = document.querySelector('#target');
+
+if (window.eventSender) {
+ eventSender.mouseMoveTo(target.offsetLeft + 10, target.offsetTop + target.offsetHeight + 10);
+ eventSender.mouseDown();
+ eventSender.mouseUp();
+ eventSender.mouseDown();
+ eventSender.mouseUp();
+}
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios/TestExpectations (282815 => 282816)
--- trunk/LayoutTests/platform/ios/TestExpectations 2021-09-21 14:54:39 UTC (rev 282815)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2021-09-21 15:05:51 UTC (rev 282816)
@@ -386,6 +386,7 @@
editing/selection/focus-and-display-none.html [ Skip ]
editing/selection/focus-crash.html [ Skip ]
editing/selection/hit-test-anonymous.html [ Skip ]
+editing/selection/hit-test-continuation-margin.html [ Skip ]
editing/selection/hit-test-on-text-with-line-height.html [ Skip ]
editing/selection/last-empty-inline.html [ Skip ]
editing/selection/minimal-user-select-crash.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (282815 => 282816)
--- trunk/Source/WebCore/ChangeLog 2021-09-21 14:54:39 UTC (rev 282815)
+++ trunk/Source/WebCore/ChangeLog 2021-09-21 15:05:51 UTC (rev 282816)
@@ -1,3 +1,24 @@
+2021-09-21 Antti Koivisto <[email protected]>
+
+ REGRESSION(r282129): Double clicking margin of a block inside a <span> may select a wrong block
+ https://bugs.webkit.org/show_bug.cgi?id=230535
+
+ Reviewed by Alan Bujtas.
+
+ If the blocks are inside a <span> we fail to search through continuation chain
+ and end up always selecting the first block inside it.
+
+ Test: editing/selection/hit-test-continuation-margin.html
+
+ * rendering/RenderInline.cpp:
+ (WebCore::RenderInline::positionForPoint):
+
+ The firstLineBox() test was really an "are inlines culled" test which stopped making sense
+ after inline culling was removed. Continuations need special handling but we would only
+ get to that code path if inlines were culled. After r282129 we would never get there.
+
+ Fix by removing the hasInlineBox test.
+
2021-09-21 Simon Fraser <[email protected]>
ScrollAnimationSmooth should only have one way to start an animation
Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (282815 => 282816)
--- trunk/Source/WebCore/rendering/RenderInline.cpp 2021-09-21 14:54:39 UTC (rev 282815)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp 2021-09-21 15:05:51 UTC (rev 282816)
@@ -425,34 +425,21 @@
VisiblePosition RenderInline::positionForPoint(const LayoutPoint& point, const RenderFragmentContainer* fragment)
{
- // FIXME: Does not deal with relative or sticky positioned inlines (should it?)
- RenderBlock& containingBlock = *this->containingBlock();
+ auto& containingBlock = *this->containingBlock();
- auto hasInlineBox = [&] {
-#if ENABLE(LAYOUT_FORMATTING_CONTEXT)
- if (LayoutIntegration::LineLayout::containing(*this))
- return true;
-#endif
- return !!firstLineBox();
- };
-
- if (hasInlineBox()) {
- // This inline actually has an inline box. We must have clicked in the border/padding of one of these boxes. We
- // should try to find a result by asking our containing block.
- return containingBlock.positionForPoint(point, fragment);
+ if (auto* continuation = this->continuation()) {
+ // Translate the coords from the pre-anonymous block to the post-anonymous block.
+ LayoutPoint parentBlockPoint = containingBlock.location() + point;
+ while (continuation) {
+ RenderBlock* currentBlock = continuation->isInline() ? continuation->containingBlock() : downcast<RenderBlock>(continuation);
+ if (continuation->isInline() || continuation->firstChild())
+ return continuation->positionForPoint(parentBlockPoint - currentBlock->locationOffset(), fragment);
+ continuation = continuation->inlineContinuation();
+ }
+ return RenderBoxModelObject::positionForPoint(point, fragment);
}
- // Translate the coords from the pre-anonymous block to the post-anonymous block.
- LayoutPoint parentBlockPoint = containingBlock.location() + point;
- RenderBoxModelObject* continuation = this->continuation();
- while (continuation) {
- RenderBlock* currentBlock = continuation->isInline() ? continuation->containingBlock() : downcast<RenderBlock>(continuation);
- if (continuation->isInline() || continuation->firstChild())
- return continuation->positionForPoint(parentBlockPoint - currentBlock->locationOffset(), fragment);
- continuation = continuation->inlineContinuation();
- }
-
- return RenderBoxModelObject::positionForPoint(point, fragment);
+ return containingBlock.positionForPoint(point, fragment);
}
class LinesBoundingBoxGeneratorContext {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
