Diff
Modified: trunk/LayoutTests/ChangeLog (261818 => 261819)
--- trunk/LayoutTests/ChangeLog 2020-05-18 17:16:16 UTC (rev 261818)
+++ trunk/LayoutTests/ChangeLog 2020-05-18 17:47:10 UTC (rev 261819)
@@ -1,3 +1,14 @@
+2020-05-18 Simon Fraser <simon.fra...@apple.com>
+
+ Find doesn't always scroll search results into view
+ https://bugs.webkit.org/show_bug.cgi?id=212007
+ <rdar://problem/36333321>
+
+ Reviewed by Wenson Hsieh.
+
+ * editing/selection/selection-bounds-fractional-containing-blocks-expected.txt: Added.
+ * editing/selection/selection-bounds-fractional-containing-blocks.html: Added.
+
2020-05-18 Ryan Haddad <ryanhad...@apple.com>
[ iOS ] fast/hidpi/filters-and-image-buffer-resolution.html is flaky failing
Added: trunk/LayoutTests/editing/selection/selection-bounds-fractional-containing-blocks-expected.txt (0 => 261819)
--- trunk/LayoutTests/editing/selection/selection-bounds-fractional-containing-blocks-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/selection-bounds-fractional-containing-blocks-expected.txt 2020-05-18 17:47:10 UTC (rev 261819)
@@ -0,0 +1,13 @@
+Tests selection bounds computation in fractionally offset renderers inside repaint containers
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS selectionBounds.width is 113
+PASS selectionBounds.height is 17
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Content
+Content
+Content
Added: trunk/LayoutTests/editing/selection/selection-bounds-fractional-containing-blocks.html (0 => 261819)
--- trunk/LayoutTests/editing/selection/selection-bounds-fractional-containing-blocks.html (rev 0)
+++ trunk/LayoutTests/editing/selection/selection-bounds-fractional-containing-blocks.html 2020-05-18 17:47:10 UTC (rev 261819)
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style>
+ .item {
+ height: 23.33px;
+ }
+ .container {
+ position: relative;
+ font-family: Ahem;
+ font-size: 12pt;
+ padding: 12.34px;
+ will-change: transform;
+ }
+</style>
+</head>
+<body>
+ <div class="container">
+ <div class="container">
+ <ol>
+ <li>
+ <div class="item">
+ Content
+ </div>
+ </li>
+ <li>
+ <div id="target" class="item">
+ Content
+ </div>
+ </li>
+ <li>
+ <div class="item">
+ Content
+ </div>
+ </li>
+ </ol>
+ </div>
+ </div>
+<script>
+description("Tests selection bounds computation in fractionally offset renderers inside repaint containers");
+
+let target = document.getElementById('target');
+let text = target.firstChild;
+
+document.getSelection().setBaseAndExtent(target, 0, target, target.childNodes.length);
+
+let selectionBounds;
+if (window.internals) {
+ selectionBounds = internals.selectionBounds();
+ shouldBe('selectionBounds.width', '113')
+ shouldBe('selectionBounds.height', '17')
+}
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios/TestExpectations (261818 => 261819)
--- trunk/LayoutTests/platform/ios/TestExpectations 2020-05-18 17:16:16 UTC (rev 261818)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2020-05-18 17:47:10 UTC (rev 261819)
@@ -1968,6 +1968,10 @@
editing/undo/undo-forward-delete-boundary.html [ Pass Failure ]
editing/undo/undo-forward-delete.html [ Pass Failure ]
+# selectionBounds() doesn't work.
+webkit.org/b/212033 editing/selection/block-cursor-overtype-mode.html [ Failure ]
+webkit.org/b/212033 editing/selection/selection-bounds-fractional-containing-blocks.html [ Failure ]
+
# storage tests that fail:
storage/websql/sql-error-codes.html [ Failure ]
Modified: trunk/LayoutTests/platform/ios-wk1/TestExpectations (261818 => 261819)
--- trunk/LayoutTests/platform/ios-wk1/TestExpectations 2020-05-18 17:16:16 UTC (rev 261818)
+++ trunk/LayoutTests/platform/ios-wk1/TestExpectations 2020-05-18 17:47:10 UTC (rev 261819)
@@ -302,7 +302,6 @@
editing/selection/anchor-focus1.html [ Failure ]
editing/selection/anchor-focus2.html [ Failure ]
editing/selection/anchor-focus3.html [ Failure ]
-editing/selection/block-cursor-overtype-mode.html [ Failure ]
editing/selection/caret-bidi-first-and-last-letters.html [ Failure ]
editing/selection/caret-in-empty-inline-1.html [ Failure ]
editing/selection/caret-in-empty-inline-2.html [ Failure ]
Modified: trunk/LayoutTests/platform/ios-wk2/TestExpectations (261818 => 261819)
--- trunk/LayoutTests/platform/ios-wk2/TestExpectations 2020-05-18 17:16:16 UTC (rev 261818)
+++ trunk/LayoutTests/platform/ios-wk2/TestExpectations 2020-05-18 17:47:10 UTC (rev 261819)
@@ -629,7 +629,6 @@
editing/secure-input/password-input-focusing.html [ Failure ]
editing/secure-input/removed-password-input.html [ Failure ]
editing/secure-input/reset-state-on-navigation.html [ Failure ]
-editing/selection/block-cursor-overtype-mode.html [ Failure ]
editing/selection/caret-in-empty-inline-1.html [ Failure ]
editing/selection/caret-in-empty-inline-2.html [ Failure ]
editing/selection/click-in-margins-inside-editable-div.html [ Failure ]
Modified: trunk/Source/WebCore/ChangeLog (261818 => 261819)
--- trunk/Source/WebCore/ChangeLog 2020-05-18 17:16:16 UTC (rev 261818)
+++ trunk/Source/WebCore/ChangeLog 2020-05-18 17:47:10 UTC (rev 261819)
@@ -1,3 +1,29 @@
+2020-05-18 Simon Fraser <simon.fra...@apple.com>
+
+ Find doesn't always scroll search results into view
+ https://bugs.webkit.org/show_bug.cgi?id=212007
+ <rdar://problem/36333321>
+
+ Reviewed by Wenson Hsieh.
+
+ HighlightData::collectBounds() could produce overly large bounds, causing the selection
+ to fail to scroll into view.
+
+ This happened when multiple block ancestors were added to 'renderers', with empty
+ rects. The process of mapping that empty rect to a quad via localToAbsoluteQuad()
+ could produce an empty quad at a fractional offset, then calling enclosingBoundingBox()
+ on the quad would create a 1x1 rectangle, which got unioned with selectionRect.
+
+ Fix by skipping entries with empty rects.
+
+ Add a Selection log channel and some logging that makes this trivial to see.
+
+ Test: editing/selection/selection-bounds-fractional-containing-blocks.html
+
+ * platform/Logging.h:
+ * rendering/HighlightData.cpp:
+ (WebCore::HighlightData::collectBounds const):
+
2020-05-18 Darin Adler <da...@apple.com>
Add iterator checking to ListHashSet
Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (261818 => 261819)
--- trunk/Source/WebCore/editing/FrameSelection.cpp 2020-05-18 17:16:16 UTC (rev 261818)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp 2020-05-18 17:47:10 UTC (rev 261819)
@@ -52,6 +52,7 @@
#include "HitTestRequest.h"
#include "HitTestResult.h"
#include "InlineTextBox.h"
+#include "Logging.h"
#include "Page.h"
#include "RenderLayer.h"
#include "RenderText.h"
@@ -68,6 +69,7 @@
#include "VisibleUnits.h"
#include <stdio.h>
#include <wtf/text/CString.h>
+#include <wtf/text/TextStream.h>
#if PLATFORM(IOS_FAMILY)
#include "Chrome.h"
@@ -404,6 +406,8 @@
void FrameSelection::setSelection(const VisibleSelection& selection, OptionSet<SetSelectionOption> options, AXTextStateChangeIntent intent, CursorAlignOnScroll align, TextGranularity granularity)
{
+ LOG_WITH_STREAM(Selection, stream << "FrameSelection::setSelection " << selection);
+
RefPtr<Document> protector(m_document);
if (!setSelectionWithoutUpdatingAppearance(selection, options, align, granularity))
return;
Modified: trunk/Source/WebCore/platform/Logging.h (261818 => 261819)
--- trunk/Source/WebCore/platform/Logging.h 2020-05-18 17:16:16 UTC (rev 261818)
+++ trunk/Source/WebCore/platform/Logging.h 2020-05-18 17:47:10 UTC (rev 261819)
@@ -96,6 +96,7 @@
M(ResourceLoadStatistics) \
M(Scrolling) \
M(ScrollLatching) \
+ M(Selection) \
M(Services) \
M(ServiceWorker) \
M(SpellingAndGrammar) \
Modified: trunk/Source/WebCore/rendering/HighlightData.cpp (261818 => 261819)
--- trunk/Source/WebCore/rendering/HighlightData.cpp 2020-05-18 17:16:16 UTC (rev 261818)
+++ trunk/Source/WebCore/rendering/HighlightData.cpp 2020-05-18 17:47:10 UTC (rev 261819)
@@ -33,6 +33,7 @@
#include "Document.h"
#include "FrameSelection.h"
+#include "Logging.h"
#include "Position.h"
#include "Range.h"
#include "RenderLayer.h"
@@ -40,6 +41,7 @@
#include "RenderObject.h"
#include "RenderView.h"
#include "VisibleSelection.h"
+#include <wtf/text/TextStream.h>
namespace WebCore {
@@ -235,11 +237,14 @@
IntRect HighlightData::collectBounds(ClipToVisibleContent clipToVisibleContent) const
{
+ LOG_WITH_STREAM(Selection, stream << "HighlightData::collectBounds (clip to visible " << (clipToVisibleContent == ClipToVisibleContent::Yes ? "yes" : "no"));
+
SelectionData::RendererMap renderers;
auto* start = m_renderRange.start();
RenderObject* stop = nullptr;
if (m_renderRange.end())
stop = rendererAfterOffset(*m_renderRange.end(), m_renderRange.endOffset().value());
+
HighlightIterator highlightIterator(start);
while (start && start != stop) {
if ((start->canBeSelectionLeaf() || start == m_renderRange.start() || start == m_renderRange.end())
@@ -246,12 +251,16 @@
&& start->selectionState() != RenderObject::HighlightState::None) {
// Blocks are responsible for painting line gaps and margin gaps. They must be examined as well.
renderers.set(start, makeUnique<RenderSelectionInfo>(*start, clipToVisibleContent == ClipToVisibleContent::Yes));
+ LOG_WITH_STREAM(Selection, stream << " added start " << *start << " with rect " << renderers.get(start)->rect());
+
auto* block = start->containingBlock();
while (block && !is<RenderView>(*block)) {
+ LOG_WITH_STREAM(Scrolling, stream << " added block " << *block);
std::unique_ptr<RenderSelectionInfo>& blockInfo = renderers.add(block, nullptr).iterator->value;
if (blockInfo)
break;
blockInfo = makeUnique<RenderSelectionInfo>(*block, clipToVisibleContent == ClipToVisibleContent::Yes);
+ LOG_WITH_STREAM(Selection, stream << " added containing block " << *block << " with rect " << blockInfo->rect());
block = block->containingBlock();
}
}
@@ -263,12 +272,19 @@
for (auto& info : renderers.values()) {
// RenderSelectionInfo::rect() is in the coordinates of the repaintContainer, so map to page coordinates.
LayoutRect currentRect = info->rect();
+ if (currentRect.isEmpty())
+ continue;
+
if (auto* repaintContainer = info->repaintContainer()) {
- FloatQuad absQuad = repaintContainer->localToAbsoluteQuad(FloatRect(currentRect));
+ FloatRect localRect = currentRect;
+ FloatQuad absQuad = repaintContainer->localToAbsoluteQuad(localRect);
currentRect = absQuad.enclosingBoundingBox();
+ LOG_WITH_STREAM(Selection, stream << " rect " << localRect << " mapped to " << currentRect << " in container " << *repaintContainer);
}
selectionRect.unite(currentRect);
}
+
+ LOG_WITH_STREAM(Selection, stream << " final rect " << selectionRect);
return snappedIntRect(selectionRect);
}
