Commit e4d44e16 authored by ojan@chromium.org's avatar ojan@chromium.org

2009-07-27 Ojan Vafai <ojan@chromium.org>

        Reviewed by Darin Adler.

        https://bugs.webkit.org/show_bug.cgi?id=27474
        Tests crashes when calling select, setSelectionRange or setting
        selectionStart/selectionEnd on a textarea/input immediately after
        setting display:none.

        * fast/dom/text-control-crash-on-select-expected.txt: Added.
        * fast/dom/text-control-crash-on-select.html: Added.

2009-07-27  Ojan Vafai  <ojan@chromium.org>

        Reviewed by Darin Adler.

        https://bugs.webkit.org/show_bug.cgi?id=27474
        Fixes crashes due to renderer getting destroyed in updateLayout.
        We need to call updateLayout before we call into the renderer.
        Removed the updateLayout call from RenderTextControl and moved it
        into the calling sites.

        Also changes updateLayout to updateLayoutIgnorePendingStylesheets so
        this works with pending stylesheets. Unfortunately, this seems to be
        untestable. Loading an external stylesheet and then having an inline
        script hit this code did not result in an pending stylesheets.

        The are other cases of this bug in the rendering code. I'll file a
        followup bug to audit the calls to updateLayout.

        Test: fast/dom/text-control-crash-on-select.html

        * dom/Document.h:
        (WebCore::Document::inStyleRecalc): Added so the ASSERTs in updateFocusAppearance
            and setSelectionRange could deal with cases of reentrancy into updateLayout
            calls. This happens in a couple layout tests.
        * dom/InputElement.cpp:
        (WebCore::InputElement::updateSelectionRange):
        * html/HTMLInputElement.cpp:
        (WebCore::isTextFieldWithRendererAfterUpdateLayout):
        (WebCore::HTMLInputElement::setSelectionStart):
        (WebCore::HTMLInputElement::setSelectionEnd):
        (WebCore::HTMLInputElement::select):
        * html/HTMLTextAreaElement.cpp:
        (WebCore::rendererAfterUpdateLayout):
        (WebCore::HTMLTextAreaElement::setSelectionStart):
        (WebCore::HTMLTextAreaElement::setSelectionEnd):
        (WebCore::HTMLTextAreaElement::select):
        (WebCore::HTMLTextAreaElement::setSelectionRange):
        (WebCore::HTMLTextAreaElement::updateFocusAppearance):
        * rendering/RenderTextControl.cpp:
        (WebCore::RenderTextControl::setSelectionRange):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@46437 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 4ac61704
2009-07-27 Ojan Vafai <ojan@chromium.org>
Reviewed by Darin Adler.
https://bugs.webkit.org/show_bug.cgi?id=27474
Tests crashes when calling select, setSelectionRange or setting
selectionStart/selectionEnd on a textarea/input immediately after
setting display:none.
* fast/dom/text-control-crash-on-select-expected.txt: Added.
* fast/dom/text-control-crash-on-select.html: Added.
2009-07-27 Nikolas Zimmermann <nikolas.zimmermann@torchmobile.com>
Reviewed by George Staikos.
......
Tests that we don't crash when killing an text input's or textarea's renderer and then calling select.
Tests that we don't crash when killing an text input's or textarea's renderer and then calling select.
<textarea id="textarea1">textarea</textarea>
<textarea id="textarea2">textarea</textarea>
<textarea id="textarea3">textarea</textarea>
<textarea id="textarea4">textarea</textarea>
<input id="input1">
<input id="input2">
<input id="input3">
<input id="input4">
<script>
if (window.layoutTestController)
layoutTestController.dumpAsText();
function $(id) {
return document.getElementById(id);
}
function testSettingSelection(tagName) {
var id = tagName + '1';
$(id).style.display = "none";
$(id).select();
id = tagName + '2';
$(id).style.display = "none";
$(id).setSelectionRange(1, 2);
id = tagName + '3';
$(id).style.display = "none";
$(id).selectionStart = 2;
id = tagName + '4';
$(id).style.display = "none";
$(id).selectionEnd = 1;
}
testSettingSelection('textarea');
testSettingSelection('input');
</script>
2009-07-27 Ojan Vafai <ojan@chromium.org>
Reviewed by Darin Adler.
https://bugs.webkit.org/show_bug.cgi?id=27474
Fixes crashes due to renderer getting destroyed in updateLayout.
We need to call updateLayout before we call into the renderer.
Removed the updateLayout call from RenderTextControl and moved it
into the calling sites.
Also changes updateLayout to updateLayoutIgnorePendingStylesheets so
this works with pending stylesheets. Unfortunately, this seems to be
untestable. Loading an external stylesheet and then having an inline
script hit this code did not result in an pending stylesheets.
The are other cases of this bug in the rendering code. I'll file a
followup bug to audit the calls to updateLayout.
Test: fast/dom/text-control-crash-on-select.html
* dom/Document.h:
(WebCore::Document::inStyleRecalc): Added so the ASSERTs in updateFocusAppearance
and setSelectionRange could deal with cases of reentrancy into updateLayout
calls. This happens in a couple layout tests.
* dom/InputElement.cpp:
(WebCore::InputElement::updateSelectionRange):
* html/HTMLInputElement.cpp:
(WebCore::isTextFieldWithRendererAfterUpdateLayout):
(WebCore::HTMLInputElement::setSelectionStart):
(WebCore::HTMLInputElement::setSelectionEnd):
(WebCore::HTMLInputElement::select):
* html/HTMLTextAreaElement.cpp:
(WebCore::rendererAfterUpdateLayout):
(WebCore::HTMLTextAreaElement::setSelectionStart):
(WebCore::HTMLTextAreaElement::setSelectionEnd):
(WebCore::HTMLTextAreaElement::select):
(WebCore::HTMLTextAreaElement::setSelectionRange):
(WebCore::HTMLTextAreaElement::updateFocusAppearance):
* rendering/RenderTextControl.cpp:
(WebCore::RenderTextControl::setSelectionRange):
2009-07-27 Dimitri Glazkov <dglazkov@chromium.org>
Reviewed by Dave Levin.
......
......@@ -1138,6 +1138,11 @@ void Document::styleRecalcTimerFired(Timer<Document>*)
updateStyleIfNeeded();
}
bool Document::childNeedsAndNotInStyleRecalc()
{
return childNeedsStyleRecalc() && !m_inStyleRecalc;
}
void Document::recalcStyle(StyleChange change)
{
// we should not enter style recalc while painting
......
......@@ -406,6 +406,7 @@ public:
PassRefPtr<EditingText> createEditingTextNode(const String&);
virtual void recalcStyle(StyleChange = NoChange);
bool childNeedsAndNotInStyleRecalc();
virtual void updateStyleIfNeeded();
void updateLayout();
void updateLayoutIgnorePendingStylesheets();
......
......@@ -116,6 +116,8 @@ void InputElement::updateSelectionRange(InputElement* inputElement, Element* ele
if (!inputElement->isTextField())
return;
element->document()->updateLayoutIgnorePendingStylesheets();
if (RenderTextControl* renderer = toRenderTextControl(element->renderer()))
renderer->setSelectionRange(start, end);
}
......
......@@ -552,31 +552,34 @@ int HTMLInputElement::selectionEnd() const
return toRenderTextControl(renderer())->selectionEnd();
}
static bool isTextFieldWithRenderer(HTMLInputElement* element)
{
if (!element->isTextField())
return false;
element->document()->updateLayoutIgnorePendingStylesheets();
if (!element->renderer())
return false;
return true;
}
void HTMLInputElement::setSelectionStart(int start)
{
if (!isTextField())
return;
if (!renderer())
return;
toRenderTextControl(renderer())->setSelectionStart(start);
if (isTextFieldWithRenderer(this))
toRenderTextControl(renderer())->setSelectionStart(start);
}
void HTMLInputElement::setSelectionEnd(int end)
{
if (!isTextField())
return;
if (!renderer())
return;
toRenderTextControl(renderer())->setSelectionEnd(end);
if (isTextFieldWithRenderer(this))
toRenderTextControl(renderer())->setSelectionEnd(end);
}
void HTMLInputElement::select()
{
if (!isTextField())
return;
if (!renderer())
return;
toRenderTextControl(renderer())->select();
if (isTextFieldWithRenderer(this))
toRenderTextControl(renderer())->select();
}
void HTMLInputElement::setSelectionRange(int start, int end)
......
......@@ -107,32 +107,34 @@ int HTMLTextAreaElement::selectionEnd()
return toRenderTextControl(renderer())->selectionEnd();
}
static RenderTextControl* rendererAfterUpdateLayout(HTMLTextAreaElement* element)
{
element->document()->updateLayoutIgnorePendingStylesheets();
return toRenderTextControl(element->renderer());
}
void HTMLTextAreaElement::setSelectionStart(int start)
{
if (!renderer())
return;
toRenderTextControl(renderer())->setSelectionStart(start);
if (RenderTextControl* renderer = rendererAfterUpdateLayout(this))
renderer->setSelectionStart(start);
}
void HTMLTextAreaElement::setSelectionEnd(int end)
{
if (!renderer())
return;
toRenderTextControl(renderer())->setSelectionEnd(end);
if (RenderTextControl* renderer = rendererAfterUpdateLayout(this))
renderer->setSelectionEnd(end);
}
void HTMLTextAreaElement::select()
{
if (!renderer())
return;
toRenderTextControl(renderer())->select();
if (RenderTextControl* renderer = rendererAfterUpdateLayout(this))
renderer->select();
}
void HTMLTextAreaElement::setSelectionRange(int start, int end)
{
if (!renderer())
return;
toRenderTextControl(renderer())->setSelectionRange(start, end);
if (RenderTextControl* renderer = rendererAfterUpdateLayout(this))
renderer->setSelectionRange(start, end);
}
void HTMLTextAreaElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
......@@ -239,7 +241,8 @@ bool HTMLTextAreaElement::isMouseFocusable() const
void HTMLTextAreaElement::updateFocusAppearance(bool restorePreviousSelection)
{
ASSERT(renderer());
ASSERT(!document()->childNeedsAndNotInStyleRecalc());
if (!restorePreviousSelection || m_cachedSelectionStart < 0) {
#if ENABLE(ON_FIRST_TEXTAREA_FOCUS_SELECT_ALL)
// Devices with trackballs or d-pads may focus on a textarea in route
......
......@@ -235,7 +235,7 @@ void RenderTextControl::setSelectionRange(int start, int end)
end = max(end, 0);
start = min(max(start, 0), end);
document()->updateLayout();
ASSERT(!document()->childNeedsAndNotInStyleRecalc());
if (style()->visibility() == HIDDEN || !m_innerText || !m_innerText->renderer() || !m_innerText->renderBox()->height()) {
cacheSelection(start, end);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment