Diff
Modified: trunk/LayoutTests/ChangeLog (201700 => 201701)
--- trunk/LayoutTests/ChangeLog 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/LayoutTests/ChangeLog 2016-06-05 19:48:26 UTC (rev 201701)
@@ -1,3 +1,14 @@
+2016-06-05 Antti Koivisto <[email protected]>
+
+ TextIterator should ignore non-visible frames in findPlainText
+ https://bugs.webkit.org/show_bug.cgi?id=158395
+
+ Reviewed by Dan Bernstein and Darin Adler.
+
+ * editing/text-iterator/count-matches-in-frames-expected.txt: Added.
+ * editing/text-iterator/count-matches-in-frames.html: Added.
+ * imported/blink/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash-width.html: Non-rendered whitespace change.
+
2016-06-04 Brady Eidson <[email protected]>
Modern IDB: Add -private.html variants of crypto/subtle IndexedDB tests.
Added: trunk/LayoutTests/editing/text-iterator/count-matches-in-frames-expected.txt (0 => 201701)
--- trunk/LayoutTests/editing/text-iterator/count-matches-in-frames-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/text-iterator/count-matches-in-frames-expected.txt 2016-06-05 19:48:26 UTC (rev 201701)
@@ -0,0 +1,12 @@
+findme0
+
+findme1
+findme2
+
+findme3
+
+PASS Search from frame in normal tree
+PASS Search from frame in display:none subtree
+PASS Search from frame in zero sized subtree
+PASS Search from frame in zero sized subtree with overflow:hidden
+
Added: trunk/LayoutTests/editing/text-iterator/count-matches-in-frames.html (0 => 201701)
--- trunk/LayoutTests/editing/text-iterator/count-matches-in-frames.html (rev 0)
+++ trunk/LayoutTests/editing/text-iterator/count-matches-in-frames.html 2016-06-05 19:48:26 UTC (rev 201701)
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Text search from frames</title>
+<script src=""
+<script src=""
+<link rel='stylesheet' href=''>
+</head>
+<body>
+
+<script>
+var count = 0;
+function findInFrame(frameHostStyle, shouldFindInFrame)
+{
+ var findString = "findme" + count++;
+ var textDiv = document.createElement("div");
+ textDiv.innerHTML = findString;
+ document.body.appendChild(textDiv)
+
+ var hostDiv = document.createElement("div");
+ hostDiv.setAttribute("style", frameHostStyle);
+ var frame = document.createElement("iframe");
+ hostDiv.appendChild(frame);
+ document.body.appendChild(hostDiv);
+
+ frame.contentDocument.body.innerHTML = findString;
+
+ assert_equals(internals.countFindMatches(findString, 0, ''), shouldFindInFrame ? 2 : 1);
+}
+
+test(function () {
+ findInFrame("", true);
+}, "Search from frame in normal tree");
+
+test(function () {
+ findInFrame("display:none", false);
+}, "Search from frame in display:none subtree");
+
+test(function () {
+ findInFrame("position:relative; width:0px; height:0px", true);
+}, "Search from frame in zero sized subtree");
+
+test(function () {
+ findInFrame("position:relative; width:0px; height:0px; border: 2px solid red; overflow:hidden", false);
+}, "Search from frame in zero sized subtree with overflow:hidden");
+
+</script>
+</html>
Modified: trunk/LayoutTests/imported/blink/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash-width-expected.txt (201700 => 201701)
--- trunk/LayoutTests/imported/blink/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash-width-expected.txt 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/LayoutTests/imported/blink/fast/shapes/shape-outside-floats/shape-outside-negative-height-crash-width-expected.txt 2016-06-05 19:48:26 UTC (rev 201701)
@@ -1,3 +1,2 @@
Test passes if it does not crash.
-
Modified: trunk/Source/WebCore/ChangeLog (201700 => 201701)
--- trunk/Source/WebCore/ChangeLog 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/Source/WebCore/ChangeLog 2016-06-05 19:48:26 UTC (rev 201701)
@@ -1,3 +1,62 @@
+2016-06-05 Antti Koivisto <[email protected]>
+
+ Find on page finds too many matches
+ https://bugs.webkit.org/show_bug.cgi?id=158395
+ rdar://problem/7440637
+
+ Reviewed by Dan Bernstein and Darin Adler.
+
+ There is a long standing bug where in some cases WebKit may find non-visible text matches when doing find on page.
+ For example searching patch review view in bugs.webkit.org returns twice as many matches as there actually are
+ on the page. This happens because the text content is replicated in an invisible subframe.
+
+ Fix by making TextIterator ignore content in non-visible subframes in findPlainText.
+
+ Test: editing/text-iterator/count-matches-in-frames.html
+
+ * editing/TextIterator.cpp:
+ (WebCore::nextInPreOrderCrossingShadowBoundaries):
+
+ Remove support for an uninteresting assertion.
+
+ (WebCore::fullyClipsContents):
+
+ Elements without renderer clip their content (except for display:contents).
+ Test the content rect instead of the size rect for emptiness.
+
+ (WebCore::ignoresContainerClip):
+ (WebCore::pushFullyClippedState):
+ (WebCore::setUpFullyClippedStack):
+ (WebCore::isClippedByFrameAncestor):
+
+ Test if the frame owner element is clipped in any of the parent frames.
+
+ (WebCore::TextIterator::TextIterator):
+
+ If the frame is clipped by its ancestors the iterator is initialized to end state.
+ Clipped frame never renders anything so there is no need to maintain clip stack and traverse.
+
+ (WebCore::findPlainText):
+
+ Use TextIteratorClipsToFrameAncestors behavior. There might be other places where
+ this behavior should be used (or perhaps it should be used always?) but limit this to
+ text search for now.
+
+ (WebCore::depthCrossingShadowBoundaries): Deleted.
+ * editing/TextIterator.h:
+ * editing/TextIteratorBehavior.h:
+
+ Add TextIteratorClipsToFrameAncestors behavior.
+
+ * testing/Internals.cpp:
+ (WebCore::Internals::countMatchesForText):
+ (WebCore::Internals::countFindMatches):
+ (WebCore::Internals::numberOfLiveNodes):
+ * testing/Internals.h:
+ * testing/Internals.idl:
+
+ Testing support
+
2016-06-05 Konstantin Tokarev <[email protected]>
Do not construct temporary copy of String from AtomicString.
Modified: trunk/Source/WebCore/editing/TextIterator.cpp (201700 => 201701)
--- trunk/Source/WebCore/editing/TextIterator.cpp 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/Source/WebCore/editing/TextIterator.cpp 2016-06-05 19:48:26 UTC (rev 201701)
@@ -33,6 +33,7 @@
#include "Frame.h"
#include "HTMLBodyElement.h"
#include "HTMLElement.h"
+#include "HTMLFrameOwnerElement.h"
#include "HTMLInputElement.h"
#include "HTMLLegendElement.h"
#include "HTMLMeterElement.h"
@@ -185,18 +186,6 @@
// --------
-#if !ASSERT_DISABLED
-
-static unsigned depthCrossingShadowBoundaries(Node& node)
-{
- unsigned depth = 0;
- for (Node* parent = node.parentOrShadowHostNode(); parent; parent = parent->parentOrShadowHostNode())
- ++depth;
- return depth;
-}
-
-#endif
-
// This function is like Range::pastLastNode, except for the fact that it can climb up out of shadow trees.
static Node* nextInPreOrderCrossingShadowBoundaries(Node& rangeEndContainer, int rangeEndOffset)
{
@@ -214,9 +203,17 @@
static inline bool fullyClipsContents(Node& node)
{
auto* renderer = node.renderer();
- if (!is<RenderBox>(renderer) || !renderer->hasOverflowClip())
+ if (!renderer) {
+ if (!is<Element>(node))
+ return false;
+ return !downcast<Element>(node).hasDisplayContents();
+ }
+ if (!is<RenderBox>(*renderer))
return false;
- return downcast<RenderBox>(*renderer).size().isEmpty();
+ auto& box = downcast<RenderBox>(*renderer);
+ if (!box.hasOverflowClip())
+ return false;
+ return box.contentSize().isEmpty();
}
static inline bool ignoresContainerClip(Node& node)
@@ -229,8 +226,6 @@
static void pushFullyClippedState(BitStack& stack, Node& node)
{
- ASSERT(stack.size() == depthCrossingShadowBoundaries(node));
-
// Push true if this node full clips its contents, or if a parent already has fully
// clipped and this is not a node that ignores its container's clip.
stack.push(fullyClipsContents(node) || (stack.top() && !ignoresContainerClip(node)));
@@ -239,6 +234,7 @@
static void setUpFullyClippedStack(BitStack& stack, Node& node)
{
// Put the nodes in a vector so we can iterate in reverse order.
+ // FIXME: This (and TextIterator in general) should use ComposedTreeIterator.
Vector<Node*, 100> ancestry;
for (Node* parent = node.parentOrShadowHostNode(); parent; parent = parent->parentOrShadowHostNode())
ancestry.append(parent);
@@ -248,8 +244,20 @@
for (size_t i = 0; i < size; ++i)
pushFullyClippedState(stack, *ancestry[size - i - 1]);
pushFullyClippedState(stack, node);
+}
- ASSERT(stack.size() == 1 + depthCrossingShadowBoundaries(node));
+static bool isClippedByFrameAncestor(const Document& document, TextIteratorBehavior behavior)
+{
+ if (!(behavior & TextIteratorClipsToFrameAncestors))
+ return false;
+
+ for (auto* owner = document.ownerElement(); owner; owner = owner->document().ownerElement()) {
+ BitStack ownerClipStack;
+ setUpFullyClippedStack(ownerClipStack, *owner);
+ if (ownerClipStack.top())
+ return true;
+ }
+ return false;
}
// FIXME: editingIgnoresContent and isRendererReplacedElement try to do the same job.
@@ -365,15 +373,17 @@
m_node = range->firstNode();
if (!m_node)
return;
+
+ if (isClippedByFrameAncestor(m_node->document(), m_behavior))
+ return;
+
setUpFullyClippedStack(m_fullyClippedStack, *m_node);
+
m_offset = m_node == m_startContainer ? m_startOffset : 0;
m_pastEndNode = nextInPreOrderCrossingShadowBoundaries(*m_endContainer, m_endOffset);
-#ifndef NDEBUG
- // Need this just because of the assert in advance().
m_positionNode = m_node;
-#endif
advance();
}
@@ -1223,10 +1233,7 @@
m_endContainer = endNode;
m_endOffset = endOffset;
-#ifndef NDEBUG
- // Need this just because of the assert.
m_positionNode = endNode;
-#endif
m_lastTextNode = nullptr;
m_lastCharacter = '\n';
@@ -2613,7 +2620,7 @@
}
}
- CharacterIterator findIterator(range, TextIteratorEntersTextControls);
+ CharacterIterator findIterator(range, TextIteratorEntersTextControls | TextIteratorClipsToFrameAncestors);
while (!findIterator.atEnd()) {
findIterator.advance(buffer.append(findIterator.text()));
@@ -2652,7 +2659,7 @@
}
// Then, find the document position of the start and the end of the text.
- CharacterIterator computeRangeIterator(range, TextIteratorEntersTextControls);
+ CharacterIterator computeRangeIterator(range, TextIteratorEntersTextControls | TextIteratorClipsToFrameAncestors);
return characterSubrange(range.ownerDocument(), computeRangeIterator, matchStart, matchLength);
}
Modified: trunk/Source/WebCore/editing/TextIteratorBehavior.h (201700 => 201701)
--- trunk/Source/WebCore/editing/TextIteratorBehavior.h 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/Source/WebCore/editing/TextIteratorBehavior.h 2016-06-05 19:48:26 UTC (rev 201701)
@@ -54,6 +54,10 @@
TextIteratorEmitsImageAltText = 1 << 6,
TextIteratorBehavesAsIfNodesFollowing = 1 << 7,
+
+ // Makes visiblity test take into account the visibility of the frame.
+ // FIXME: This should probably be always on unless TextIteratorIgnoresStyleVisibility is set.
+ TextIteratorClipsToFrameAncestors = 1 << 8,
};
typedef unsigned short TextIteratorBehavior;
Modified: trunk/Source/WebCore/rendering/RenderBox.h (201700 => 201701)
--- trunk/Source/WebCore/rendering/RenderBox.h 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/Source/WebCore/rendering/RenderBox.h 2016-06-05 19:48:26 UTC (rev 201701)
@@ -209,6 +209,7 @@
void updateLayerTransform();
+ LayoutSize contentSize() const { return { contentWidth(), contentHeight() }; }
LayoutUnit contentWidth() const { return clientWidth() - paddingLeft() - paddingRight(); }
LayoutUnit contentHeight() const { return clientHeight() - paddingTop() - paddingBottom(); }
LayoutUnit contentLogicalWidth() const { return style().isHorizontalWritingMode() ? contentWidth() : contentHeight(); }
Modified: trunk/Source/WebCore/testing/Internals.cpp (201700 => 201701)
--- trunk/Source/WebCore/testing/Internals.cpp 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/Source/WebCore/testing/Internals.cpp 2016-06-05 19:48:26 UTC (rev 201701)
@@ -1733,6 +1733,15 @@
return document->frame()->editor().countMatchesForText(text, nullptr, findOptions, 1000, mark, nullptr);
}
+unsigned Internals::countFindMatches(const String& text, unsigned findOptions, ExceptionCode&)
+{
+ Document* document = contextDocument();
+ if (!document || !document->page())
+ return 0;
+
+ return document->page()->countFindMatches(text, findOptions, 1000);
+}
+
unsigned Internals::numberOfLiveNodes() const
{
unsigned nodeCount = 0;
Modified: trunk/Source/WebCore/testing/Internals.h (201700 => 201701)
--- trunk/Source/WebCore/testing/Internals.h 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/Source/WebCore/testing/Internals.h 2016-06-05 19:48:26 UTC (rev 201701)
@@ -224,6 +224,7 @@
void toggleOverwriteModeEnabled(ExceptionCode&);
unsigned countMatchesForText(const String&, unsigned findOptions, const String& markMatches, ExceptionCode&);
+ unsigned countFindMatches(const String&, unsigned findOptions, ExceptionCode&);
unsigned numberOfScrollableAreas(ExceptionCode&);
Modified: trunk/Source/WebCore/testing/Internals.idl (201700 => 201701)
--- trunk/Source/WebCore/testing/Internals.idl 2016-06-05 19:38:57 UTC (rev 201700)
+++ trunk/Source/WebCore/testing/Internals.idl 2016-06-05 19:48:26 UTC (rev 201701)
@@ -161,6 +161,7 @@
void setAutofilled(HTMLInputElement inputElement, boolean enabled);
void setShowAutoFillButton(HTMLInputElement inputElement, AutoFillButtonType autoFillButtonType);
[RaisesException] unsigned long countMatchesForText(DOMString text, unsigned long findOptions, DOMString markMatches);
+ [RaisesException] unsigned long countFindMatches(DOMString text, unsigned long findOptions);
[RaisesException] DOMString autofillFieldName(Element formControlElement);
