- Revision
- 94274
- Author
- [email protected]
- Date
- 2011-08-31 23:11:07 -0700 (Wed, 31 Aug 2011)
Log Message
Crash when inserting text with a trailing newline into a textarea via JS
https://bugs.webkit.org/show_bug.cgi?id=66241
Reviewed by Darin Adler and Kent Tamura.
Source/WebCore:
The crash was caused by updateFromElement biting on the editing code.
When there is a style rule that applies on text nodes inside the shadow DOM, DOM modifications made
by the editing code may trigger style recalculation on input or textarea elements in the midst of editing
commands. In response to this style recalculation, HTMLInputElement::updateFromElement and
HTMLTextAreaElement::updateFromElement call setInnerTextValue to re-create the text nodes in the
shadow DOM. The editing code blows up because setInnerTextValue detaches old text nodes referenced by
Positions and VisiblePositions held by the editing commands in progress.
Fixed the crash by stop calling setInnerTextValue in updateFromElement. Instead, WebKit now creates
the text nodes when attributes, descendent nodes, etc... of input or textarea element changes.
Tests: fast/forms/update-from-element-during-editing-crash-1.html
fast/forms/update-from-element-during-editing-crash-2.html
* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::updateType): Force calling setInnerTextValue when input type changes.
(WebCore::HTMLInputElement::updateInnerTextValue): Extracted from RenderTextControlSingleLine's
updateElement.
(WebCore::HTMLInputElement::parseMappedAttribute): Calls updateInnerTextValue; force calling
setInnerTextValue when -webkit-speech attribute changes. In the theory, we should be able to call it less
frequently but there are too many cases to consider at the moment.
(WebCore::HTMLInputElement::setValue): Calls updateInnerTextValue when the value actually changed.
Note we need to call it before we set or restore selection.
* html/HTMLInputElement.h:
* html/HTMLTextAreaElement.cpp:
(WebCore::HTMLTextAreaElement::childrenChanged): Calls updateInnerTextValue when textarea's descendants
nodes are changed by parser or scripts.
(WebCore::HTMLTextAreaElement::setValueCommon): Calls updateInnerTextValue when the value changes.
* html/HTMLTextFormControlElement.h:
* html/NumberInputType.cpp:
(WebCore::NumberInputType::willBlur): Calls updateInnerTextValue because input[type=number] forces
the value to be valid on blur.
* rendering/RenderTextControlMultiLine.cpp: Removed RenderTextControlMultiLine::updateFromElement.
* rendering/RenderTextControlMultiLine.h: Ditto.
* rendering/RenderTextControlSingleLine.cpp:
(WebCore::RenderTextControlSingleLine::updateFromElement):
LayoutTests:
Add regressions tests to ensure WebKit doesn't crash even when style recalc happens
in the midst of editing commands.
* fast/forms/update-from-element-during-editing-crash-1-expected.txt: Added.
* fast/forms/update-from-element-during-editing-crash-1.html: Added.
* fast/forms/update-from-element-during-editing-crash-2-expected.txt: Added.
* fast/forms/update-from-element-during-editing-crash-2.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (94273 => 94274)
--- trunk/LayoutTests/ChangeLog 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/LayoutTests/ChangeLog 2011-09-01 06:11:07 UTC (rev 94274)
@@ -1,3 +1,18 @@
+2011-08-31 Ryosuke Niwa <[email protected]>
+
+ Crash when inserting text with a trailing newline into a textarea via JS
+ https://bugs.webkit.org/show_bug.cgi?id=66241
+
+ Reviewed by Darin Adler and Kent Tamura.
+
+ Add regressions tests to ensure WebKit doesn't crash even when style recalc happens
+ in the midst of editing commands.
+
+ * fast/forms/update-from-element-during-editing-crash-1-expected.txt: Added.
+ * fast/forms/update-from-element-during-editing-crash-1.html: Added.
+ * fast/forms/update-from-element-during-editing-crash-2-expected.txt: Added.
+ * fast/forms/update-from-element-during-editing-crash-2.html: Added.
+
2011-08-31 David Levin <[email protected]>
[chromium] Add baselines due to r94253 and r94258.
Added: trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-1-expected.txt (0 => 94274)
--- trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-1-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-1-expected.txt 2011-09-01 06:11:07 UTC (rev 94274)
@@ -0,0 +1,2 @@
+This test verifies WebKit doesn't crash even if the DOM changes in shadow DOM caused the shadow host's style to change.
+PASS
Added: trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-1.html (0 => 94274)
--- trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-1.html (rev 0)
+++ trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-1.html 2011-09-01 06:11:07 UTC (rev 94274)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+#container + * { clear: both; }
+</style>
+<p id="container"><textarea></textarea></p>
+<script>
+
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+var textarea = document.getElementsByTagName('textarea')[0];
+textarea.focus();
+textarea.innerHTML = 'a\n';
+textarea.selectionStart = 1;
+textarea.selectionEnd = 1;
+document.execCommand('InsertLineBreak', false, null);
+
+document.body.innerText = "This test verifies WebKit doesn't crash even if the DOM changes in shadow DOM caused the shadow host's style to change.\nPASS";
+
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-2-expected.txt (0 => 94274)
--- trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-2-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-2-expected.txt 2011-09-01 06:11:07 UTC (rev 94274)
@@ -0,0 +1,2 @@
+This test verifies WebKit doesn't crash even if the DOM changes in shadow DOM caused the shadow host's style to change.
+PASS
Added: trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-2.html (0 => 94274)
--- trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-2.html (rev 0)
+++ trunk/LayoutTests/fast/forms/update-from-element-during-editing-crash-2.html 2011-09-01 06:11:07 UTC (rev 94274)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+div + * {}
+</style>
+<p id="container"><textarea>a
+</textarea></p>
+<script>
+
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+var textarea = document.getElementsByTagName('textarea')[0];
+textarea.focus();
+textarea.selectionStart = 1;
+textarea.selectionEnd = 1;
+document.execCommand('InsertLineBreak', false, null);
+
+document.body.innerText = "This test verifies WebKit doesn't crash even if the DOM changes in shadow DOM caused the shadow host's style to change.\nPASS";
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (94273 => 94274)
--- trunk/Source/WebCore/ChangeLog 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/ChangeLog 2011-09-01 06:11:07 UTC (rev 94274)
@@ -1,3 +1,48 @@
+2011-08-31 Ryosuke Niwa <[email protected]>
+
+ Crash when inserting text with a trailing newline into a textarea via JS
+ https://bugs.webkit.org/show_bug.cgi?id=66241
+
+ Reviewed by Darin Adler and Kent Tamura.
+
+ The crash was caused by updateFromElement biting on the editing code.
+
+ When there is a style rule that applies on text nodes inside the shadow DOM, DOM modifications made
+ by the editing code may trigger style recalculation on input or textarea elements in the midst of editing
+ commands. In response to this style recalculation, HTMLInputElement::updateFromElement and
+ HTMLTextAreaElement::updateFromElement call setInnerTextValue to re-create the text nodes in the
+ shadow DOM. The editing code blows up because setInnerTextValue detaches old text nodes referenced by
+ Positions and VisiblePositions held by the editing commands in progress.
+
+ Fixed the crash by stop calling setInnerTextValue in updateFromElement. Instead, WebKit now creates
+ the text nodes when attributes, descendent nodes, etc... of input or textarea element changes.
+
+ Tests: fast/forms/update-from-element-during-editing-crash-1.html
+ fast/forms/update-from-element-during-editing-crash-2.html
+
+ * html/HTMLInputElement.cpp:
+ (WebCore::HTMLInputElement::updateType): Force calling setInnerTextValue when input type changes.
+ (WebCore::HTMLInputElement::updateInnerTextValue): Extracted from RenderTextControlSingleLine's
+ updateElement.
+ (WebCore::HTMLInputElement::parseMappedAttribute): Calls updateInnerTextValue; force calling
+ setInnerTextValue when -webkit-speech attribute changes. In the theory, we should be able to call it less
+ frequently but there are too many cases to consider at the moment.
+ (WebCore::HTMLInputElement::setValue): Calls updateInnerTextValue when the value actually changed.
+ Note we need to call it before we set or restore selection.
+ * html/HTMLInputElement.h:
+ * html/HTMLTextAreaElement.cpp:
+ (WebCore::HTMLTextAreaElement::childrenChanged): Calls updateInnerTextValue when textarea's descendants
+ nodes are changed by parser or scripts.
+ (WebCore::HTMLTextAreaElement::setValueCommon): Calls updateInnerTextValue when the value changes.
+ * html/HTMLTextFormControlElement.h:
+ * html/NumberInputType.cpp:
+ (WebCore::NumberInputType::willBlur): Calls updateInnerTextValue because input[type=number] forces
+ the value to be valid on blur.
+ * rendering/RenderTextControlMultiLine.cpp: Removed RenderTextControlMultiLine::updateFromElement.
+ * rendering/RenderTextControlMultiLine.h: Ditto.
+ * rendering/RenderTextControlSingleLine.cpp:
+ (WebCore::RenderTextControlSingleLine::updateFromElement):
+
2011-08-31 Tom Zakrajsek <[email protected]>
Fix snow-leopard regression caused by r93982
Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (94273 => 94274)
--- trunk/Source/WebCore/html/HTMLInputElement.cpp 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp 2011-09-01 06:11:07 UTC (rev 94274)
@@ -49,7 +49,6 @@
#include "NumberInputType.h"
#include "RenderTextControlSingleLine.h"
#include "RenderTheme.h"
-#include "RuntimeEnabledFeatures.h"
#include "SearchInputType.h"
#include "ScriptEventListener.h"
#include "WheelEvent.h"
@@ -61,6 +60,10 @@
#include "ColorInputType.h"
#endif
+#if ENABLE(INPUT_SPEECH)
+#include "RuntimeEnabledFeatures.h"
+#endif
+
using namespace std;
namespace WebCore {
@@ -583,6 +586,10 @@
m_valueIfDirty = sanitizeValue(fastGetAttribute(valueAttr));
else
updateValueIfNeeded();
+
+ setFormControlValueMatchesRenderer(false);
+ updateInnerTextValue();
+
m_wasModifiedByUser = false;
if (neededActivationCallback)
@@ -615,6 +622,20 @@
notifyFormStateChanged();
}
+void HTMLInputElement::updateInnerTextValue()
+{
+ if (!isTextField())
+ return;
+
+ if (!suggestedValue().isNull())
+ setInnerTextValue(suggestedValue());
+ else if (!formControlValueMatchesRenderer()) {
+ // Update the renderer value if the formControlValueMatchesRenderer() flag is false.
+ // It protects an unacceptable renderer value from being overwritten with the DOM value.
+ setInnerTextValue(visibleValue());
+ }
+}
+
void HTMLInputElement::subtreeHasChanged()
{
ASSERT(isTextField());
@@ -822,12 +843,14 @@
m_inputType->destroyShadowSubtree();
m_inputType->createShadowSubtree();
}
+ setFormControlValueMatchesRenderer(false);
setNeedsStyleRecalc();
} else if (attr->name() == onwebkitspeechchangeAttr)
setAttributeEventListener(eventNames().webkitspeechchangeEvent, createAttributeEventListener(this, attr));
#endif
else
HTMLTextFormControlElement::parseMappedAttribute(attr);
+ updateInnerTextValue();
}
void HTMLInputElement::finishParsingChildren()
@@ -1077,6 +1100,9 @@
setNeedsValidityCheck();
+ if (valueChanged)
+ updateInnerTextValue();
+
if (isTextField()) {
unsigned max = visibleValue().length();
if (document()->focusedNode() == this)
Modified: trunk/Source/WebCore/html/HTMLInputElement.h (94273 => 94274)
--- trunk/Source/WebCore/html/HTMLInputElement.h 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/html/HTMLInputElement.h 2011-09-01 06:11:07 UTC (rev 94274)
@@ -145,6 +145,8 @@
String sanitizeValue(const String&) const;
+ void updateInnerTextValue();
+
// The value which is drawn by a renderer.
String visibleValue() const;
String convertFromVisibleValue(const String&) const;
Modified: trunk/Source/WebCore/html/HTMLTextAreaElement.cpp (94273 => 94274)
--- trunk/Source/WebCore/html/HTMLTextAreaElement.cpp 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/html/HTMLTextAreaElement.cpp 2011-09-01 06:11:07 UTC (rev 94274)
@@ -36,7 +36,6 @@
#include "FormDataList.h"
#include "Frame.h"
#include "HTMLNames.h"
-#include "RenderStyle.h"
#include "RenderTextControlMultiLine.h"
#include "ShadowRoot.h"
#include "Text.h"
@@ -101,6 +100,7 @@
setLastChangeWasNotUserEdit();
if (!m_isDirty)
setNonDirtyValue(defaultValue());
+ setInnerTextValue(value());
HTMLElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
}
@@ -329,6 +329,7 @@
return;
m_value = normalizedValue;
+ setInnerTextValue(m_value);
setLastChangeWasNotUserEdit();
updatePlaceholderVisibility(false);
setNeedsStyleRecalc();
Modified: trunk/Source/WebCore/html/HTMLTextFormControlElement.h (94273 => 94274)
--- trunk/Source/WebCore/html/HTMLTextFormControlElement.h 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/html/HTMLTextFormControlElement.h 2011-09-01 06:11:07 UTC (rev 94274)
@@ -103,6 +103,7 @@
void setLastChangeWasNotUserEdit() { m_lastChangeWasUserEdit = false; }
String valueWithHardLineBreaks() const;
+
private:
int computeSelectionStart() const;
int computeSelectionEnd() const;
Modified: trunk/Source/WebCore/html/NumberInputType.cpp (94273 => 94274)
--- trunk/Source/WebCore/html/NumberInputType.cpp 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/html/NumberInputType.cpp 2011-09-01 06:11:07 UTC (rev 94274)
@@ -280,8 +280,7 @@
// We need to reset the renderer value explicitly because an unacceptable
// renderer value should be purged before style calculation.
- if (element()->renderer())
- element()->renderer()->updateFromElement();
+ element()->updateInnerTextValue();
}
String NumberInputType::visibleValue() const
Modified: trunk/Source/WebCore/rendering/RenderTextControlMultiLine.cpp (94273 => 94274)
--- trunk/Source/WebCore/rendering/RenderTextControlMultiLine.cpp 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/rendering/RenderTextControlMultiLine.cpp 2011-09-01 06:11:07 UTC (rev 94274)
@@ -80,13 +80,6 @@
return RenderBox::baselinePosition(baselineType, firstLine, direction, linePositionMode);
}
-void RenderTextControlMultiLine::updateFromElement()
-{
- RenderTextControl::updateFromElement();
-
- textFormControlElement()->setInnerTextValue(static_cast<HTMLTextAreaElement*>(node())->value());
-}
-
PassRefPtr<RenderStyle> RenderTextControlMultiLine::createInnerTextStyle(const RenderStyle* startStyle) const
{
RefPtr<RenderStyle> textBlockStyle = RenderStyle::create();
Modified: trunk/Source/WebCore/rendering/RenderTextControlMultiLine.h (94273 => 94274)
--- trunk/Source/WebCore/rendering/RenderTextControlMultiLine.h 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/rendering/RenderTextControlMultiLine.h 2011-09-01 06:11:07 UTC (rev 94274)
@@ -41,8 +41,6 @@
virtual void adjustControlHeightBasedOnLineHeight(LayoutUnit lineHeight);
virtual LayoutUnit baselinePosition(FontBaseline, bool firstLine, LineDirectionMode, LinePositionMode = PositionOnContainingLine) const;
- virtual void updateFromElement();
-
virtual RenderStyle* textBaseStyle() const;
virtual PassRefPtr<RenderStyle> createInnerTextStyle(const RenderStyle* startStyle) const;
virtual RenderObject* layoutSpecialExcludedChild(bool relayoutChildren);
Modified: trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp (94273 => 94274)
--- trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp 2011-09-01 06:07:32 UTC (rev 94273)
+++ trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp 2011-09-01 06:11:07 UTC (rev 94274)
@@ -476,18 +476,6 @@
if (cancelButtonElement())
updateCancelButtonVisibility();
- if (!inputElement()->suggestedValue().isNull())
- textFormControlElement()->setInnerTextValue(inputElement()->suggestedValue());
- else {
- if (node()->hasTagName(inputTag)) {
- // For HTMLInputElement, update the renderer value if the formControlValueMatchesRenderer()
- // flag is false. It protects an unacceptable renderer value from
- // being overwritten with the DOM value.
- if (!inputElement()->formControlValueMatchesRenderer())
- textFormControlElement()->setInnerTextValue(inputElement()->visibleValue());
- }
- }
-
if (m_searchPopupIsVisible)
m_searchPopup->popupMenu()->updateFromElement();
}
