Diff
Modified: trunk/LayoutTests/ChangeLog (88455 => 88456)
--- trunk/LayoutTests/ChangeLog 2011-06-09 16:05:08 UTC (rev 88455)
+++ trunk/LayoutTests/ChangeLog 2011-06-09 16:15:29 UTC (rev 88456)
@@ -1,3 +1,13 @@
+2011-06-08 Abhishek Arya <[email protected]>
+
+ Reviewed by Ryosuke Niwa.
+
+ Tests that setting selection on a text control does not result in crash.
+ https://bugs.webkit.org/show_bug.cgi?id=62329
+
+ * fast/forms/text-control-selection-crash-expected.txt: Added.
+ * fast/forms/text-control-selection-crash.html: Added.
+
2011-06-09 Csaba Osztrogonác <[email protected]>
[Qt] Unreviewed fix after r88447.
Added: trunk/LayoutTests/fast/forms/text-control-selection-crash-expected.txt (0 => 88456)
--- trunk/LayoutTests/fast/forms/text-control-selection-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/text-control-selection-crash-expected.txt 2011-06-09 16:15:29 UTC (rev 88456)
@@ -0,0 +1 @@
+Test passes if it does not crash.
Added: trunk/LayoutTests/fast/forms/text-control-selection-crash.html (0 => 88456)
--- trunk/LayoutTests/fast/forms/text-control-selection-crash.html (rev 0)
+++ trunk/LayoutTests/fast/forms/text-control-selection-crash.html 2011-06-09 16:15:29 UTC (rev 88456)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+Test passes if it does not crash.
+<textarea id="A"></textarea>
+<textarea id="B"></textarea>
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+A.selectionStart = 0;
+B.style.display = "none";
+B.selectionStart = 0;
+</script>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (88455 => 88456)
--- trunk/Source/WebCore/ChangeLog 2011-06-09 16:05:08 UTC (rev 88455)
+++ trunk/Source/WebCore/ChangeLog 2011-06-09 16:15:29 UTC (rev 88456)
@@ -1,3 +1,25 @@
+2011-06-08 Abhishek Arya <[email protected]>
+
+ Reviewed by Ryosuke Niwa.
+
+ Make indexForVisiblePosition and isSelectableElement static.
+ https://bugs.webkit.org/show_bug.cgi?id=62329
+
+ This protects us when converting frame->selection->start() or end()
+ to VisiblePosition which blows away the RenderTextControl from
+ underneath (due to layout update).
+
+ Test: fast/forms/text-control-selection-crash.html
+
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::indexForVisiblePosition):
+ * rendering/RenderTextControl.cpp:
+ (WebCore::RenderTextControl::selectionStart):
+ (WebCore::RenderTextControl::selectionEnd):
+ (WebCore::RenderTextControl::isSelectableElement):
+ (WebCore::RenderTextControl::indexForVisiblePosition):
+ * rendering/RenderTextControl.h:
+
2011-06-09 Ben Murdoch <[email protected]>
Reviewed by Yury Semikhatsky.
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (88455 => 88456)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2011-06-09 16:05:08 UTC (rev 88455)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2011-06-09 16:15:29 UTC (rev 88456)
@@ -2499,7 +2499,7 @@
int AccessibilityRenderObject::indexForVisiblePosition(const VisiblePosition& pos) const
{
if (isNativeTextControl())
- return toRenderTextControl(m_renderer)->indexForVisiblePosition(pos);
+ return RenderTextControl::indexForVisiblePosition(toRenderTextControl(m_renderer)->innerTextElement(), pos);
if (!isTextControl())
return 0;
Modified: trunk/Source/WebCore/rendering/RenderTextControl.cpp (88455 => 88456)
--- trunk/Source/WebCore/rendering/RenderTextControl.cpp 2011-06-09 16:05:08 UTC (rev 88455)
+++ trunk/Source/WebCore/rendering/RenderTextControl.cpp 2011-06-09 16:15:29 UTC (rev 88456)
@@ -177,7 +177,12 @@
Frame* frame = this->frame();
if (!frame)
return 0;
- return indexForVisiblePosition(frame->selection()->start());
+
+ HTMLElement* innerText = innerTextElement();
+ // Do not call innerTextElement() in the function arguments as creating a VisiblePosition
+ // from frame->selection->start() can blow us from underneath. Also, function ordering is
+ // usually dependent on the compiler.
+ return RenderTextControl::indexForVisiblePosition(innerText, frame->selection()->start());
}
int RenderTextControl::selectionEnd() const
@@ -185,7 +190,12 @@
Frame* frame = this->frame();
if (!frame)
return 0;
- return indexForVisiblePosition(frame->selection()->end());
+
+ HTMLElement* innerText = innerTextElement();
+ // Do not call innerTextElement() in the function arguments as creating a VisiblePosition
+ // from frame->selection->end() can blow us from underneath. Also, function ordering is
+ // usually dependent on the compiler.
+ return RenderTextControl::indexForVisiblePosition(innerText, frame->selection()->end());
}
bool RenderTextControl::hasVisibleTextArea() const
@@ -229,15 +239,11 @@
frame->selection()->setSelection(newSelection);
}
-bool RenderTextControl::isSelectableElement(Node* node) const
+bool RenderTextControl::isSelectableElement(HTMLElement* innerText, Node* node)
{
- if (!node)
+ if (!node || !innerText)
return false;
- HTMLElement* innerText = innerTextElement();
- if (!innerText)
- return false;
-
if (node->rootEditableElement() == innerText)
return true;
@@ -312,14 +318,14 @@
return VisiblePosition(Position(endContainer, endOffset, Position::PositionIsOffsetInAnchor), UPSTREAM);
}
-int RenderTextControl::indexForVisiblePosition(const VisiblePosition& pos) const
+int RenderTextControl::indexForVisiblePosition(HTMLElement* innerTextElement, const VisiblePosition& pos)
{
Position indexPosition = pos.deepEquivalent();
- if (!isSelectableElement(indexPosition.deprecatedNode()))
+ if (!RenderTextControl::isSelectableElement(innerTextElement, indexPosition.deprecatedNode()))
return 0;
ExceptionCode ec = 0;
- RefPtr<Range> range = Range::create(document());
- range->setStart(innerTextElement(), 0, ec);
+ RefPtr<Range> range = Range::create(indexPosition.document());
+ range->setStart(innerTextElement, 0, ec);
ASSERT(!ec);
range->setEnd(indexPosition.deprecatedNode(), indexPosition.deprecatedEditingOffset(), ec);
ASSERT(!ec);
Modified: trunk/Source/WebCore/rendering/RenderTextControl.h (88455 => 88456)
--- trunk/Source/WebCore/rendering/RenderTextControl.h 2011-06-09 16:05:08 UTC (rev 88455)
+++ trunk/Source/WebCore/rendering/RenderTextControl.h 2011-06-09 16:15:29 UTC (rev 88456)
@@ -50,7 +50,7 @@
void selectionChanged(bool userTriggered);
VisiblePosition visiblePositionForIndex(int index) const;
- int indexForVisiblePosition(const VisiblePosition&) const;
+ static int indexForVisiblePosition(HTMLElement*, const VisiblePosition&);
void updatePlaceholderVisibility(bool, bool);
@@ -102,7 +102,7 @@
bool hasVisibleTextArea() const;
friend void setSelectionRange(Node*, int start, int end);
- bool isSelectableElement(Node*) const;
+ static bool isSelectableElement(HTMLElement*, Node*);
virtual int textBlockInsetLeft() const = 0;
virtual int textBlockInsetRight() const = 0;
