Title: [184308] trunk
- Revision
- 184308
- Author
- [email protected]
- Date
- 2015-05-13 14:58:41 -0700 (Wed, 13 May 2015)
Log Message
REGRESSION(r183770): Crash inside WebEditorClient::shouldApplyStyle when applying underline
https://bugs.webkit.org/show_bug.cgi?id=144949
Source/WebCore:
<rdar://problem/20895753>
Reviewed by Darin Adler.
The crash was caused by the variant of applyStyleToSelection that takes EditingStyle passing
a null pointer to shouldApplyStyle when we're only applying text decoration changes so that
m_mutableStyle in the editing style is null. This didn't reproduce in execCommand since we
wouldn't call shouldApplyStyle in that case. It didn't reproduce in my manual testing because
font panel also sets text shadow, which ends up filling up m_mutableStyle.
Fixed the bug by creating a mutable style properties when one is not provided by EditingStyle.
Also fixed the "FIXME" in the function by converting text decoration changes to a corresponding
text decoration value. The values passed to shouldApplyStyle now matches the old behavior prior
to r183770.
Test: editing/style/underline-by-user.html
* editing/EditingStyle.cpp:
(WebCore::EditingStyle::styleWithResolvedTextDecorations): Added.
* editing/EditingStyle.h:
* editing/Editor.cpp:
(WebCore::Editor::applyStyleToSelection): Use styleWithResolvedTextDecorations to avoid the crash.
LayoutTests:
Reviewed by Darin Adler.
Added a test that emulates underlining of text by the user. Unlike document.execCommand,
testRunner.execCommand simulates a user initiated editing command and therefore invokes
shouldApplyStyle.
* editing/style/underline-by-user-expected.txt: Added.
* editing/style/underline-by-user.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (184307 => 184308)
--- trunk/LayoutTests/ChangeLog 2015-05-13 21:37:50 UTC (rev 184307)
+++ trunk/LayoutTests/ChangeLog 2015-05-13 21:58:41 UTC (rev 184308)
@@ -1,3 +1,17 @@
+2015-05-13 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r183770): Crash inside WebEditorClient::shouldApplyStyle when applying underline
+ https://bugs.webkit.org/show_bug.cgi?id=144949
+
+ Reviewed by Darin Adler.
+
+ Added a test that emulates underlining of text by the user. Unlike document.execCommand,
+ testRunner.execCommand simulates a user initiated editing command and therefore invokes
+ shouldApplyStyle.
+
+ * editing/style/underline-by-user-expected.txt: Added.
+ * editing/style/underline-by-user.html: Added.
+
2015-05-13 Yusuke Suzuki <[email protected]>
[ES6] Implement String.raw
Added: trunk/LayoutTests/editing/style/underline-by-user-expected.txt (0 => 184308)
--- trunk/LayoutTests/editing/style/underline-by-user-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/style/underline-by-user-expected.txt 2015-05-13 21:58:41 UTC (rev 184308)
@@ -0,0 +1,3 @@
+This tests user initialized underlining of text. To manually test, underline "hello" below. WebKit should not crash.
+
+hello
Added: trunk/LayoutTests/editing/style/underline-by-user.html (0 => 184308)
--- trunk/LayoutTests/editing/style/underline-by-user.html (rev 0)
+++ trunk/LayoutTests/editing/style/underline-by-user.html 2015-05-13 21:58:41 UTC (rev 184308)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests user initialized underlining of text. To manually test, underline "hello" below. WebKit should not crash.</p>
+<div id="editor" contenteditable>hello</div>
+<script>
+document.getElementById('editor').focus();
+document.execCommand('selectAll', false, null);
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.execCommand('underline', false, null);
+}
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (184307 => 184308)
--- trunk/Source/WebCore/ChangeLog 2015-05-13 21:37:50 UTC (rev 184307)
+++ trunk/Source/WebCore/ChangeLog 2015-05-13 21:58:41 UTC (rev 184308)
@@ -1,3 +1,30 @@
+2015-05-13 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r183770): Crash inside WebEditorClient::shouldApplyStyle when applying underline
+ https://bugs.webkit.org/show_bug.cgi?id=144949
+ <rdar://problem/20895753>
+
+ Reviewed by Darin Adler.
+
+ The crash was caused by the variant of applyStyleToSelection that takes EditingStyle passing
+ a null pointer to shouldApplyStyle when we're only applying text decoration changes so that
+ m_mutableStyle in the editing style is null. This didn't reproduce in execCommand since we
+ wouldn't call shouldApplyStyle in that case. It didn't reproduce in my manual testing because
+ font panel also sets text shadow, which ends up filling up m_mutableStyle.
+
+ Fixed the bug by creating a mutable style properties when one is not provided by EditingStyle.
+ Also fixed the "FIXME" in the function by converting text decoration changes to a corresponding
+ text decoration value. The values passed to shouldApplyStyle now matches the old behavior prior
+ to r183770.
+
+ Test: editing/style/underline-by-user.html
+
+ * editing/EditingStyle.cpp:
+ (WebCore::EditingStyle::styleWithResolvedTextDecorations): Added.
+ * editing/EditingStyle.h:
+ * editing/Editor.cpp:
+ (WebCore::Editor::applyStyleToSelection): Use styleWithResolvedTextDecorations to avoid the crash.
+
2015-05-13 Eric Carlson <[email protected]>
Work around HTMLMediaElement::documentDidResumeFromPageCache being called twice
Modified: trunk/Source/WebCore/editing/EditingStyle.cpp (184307 => 184308)
--- trunk/Source/WebCore/editing/EditingStyle.cpp 2015-05-13 21:37:50 UTC (rev 184307)
+++ trunk/Source/WebCore/editing/EditingStyle.cpp 2015-05-13 21:58:41 UTC (rev 184308)
@@ -525,6 +525,28 @@
&& underlineChange() == TextDecorationChange::None && strikeThroughChange() == TextDecorationChange::None;
}
+Ref<MutableStyleProperties> EditingStyle::styleWithResolvedTextDecorations() const
+{
+ bool hasTextDecorationChanges = underlineChange() != TextDecorationChange::None || strikeThroughChange() != TextDecorationChange::None;
+ if (m_mutableStyle && !hasTextDecorationChanges)
+ return *m_mutableStyle;
+
+ Ref<MutableStyleProperties> style = m_mutableStyle ? m_mutableStyle->mutableCopy() : MutableStyleProperties::create();
+
+ Ref<CSSValueList> valueList = CSSValueList::createSpaceSeparated();
+ if (underlineChange() == TextDecorationChange::Add)
+ valueList->append(cssValuePool().createIdentifierValue(CSSValueUnderline));
+ if (strikeThroughChange() == TextDecorationChange::Add)
+ valueList->append(cssValuePool().createIdentifierValue(CSSValueLineThrough));
+
+ if (valueList->length())
+ style->setProperty(CSSPropertyTextDecoration, valueList.ptr());
+ else
+ style->setProperty(CSSPropertyTextDecoration, cssValuePool().createIdentifierValue(CSSValueNone));
+
+ return style;
+}
+
bool EditingStyle::textDirection(WritingDirection& writingDirection) const
{
if (!m_mutableStyle)
Modified: trunk/Source/WebCore/editing/EditingStyle.h (184307 => 184308)
--- trunk/Source/WebCore/editing/EditingStyle.h 2015-05-13 21:37:50 UTC (rev 184307)
+++ trunk/Source/WebCore/editing/EditingStyle.h 2015-05-13 21:58:41 UTC (rev 184308)
@@ -112,6 +112,7 @@
WEBCORE_EXPORT ~EditingStyle();
MutableStyleProperties* style() { return m_mutableStyle.get(); }
+ Ref<MutableStyleProperties> styleWithResolvedTextDecorations() const;
bool textDirection(WritingDirection&) const;
bool isEmpty() const;
void setStyle(PassRefPtr<MutableStyleProperties>);
Modified: trunk/Source/WebCore/editing/Editor.cpp (184307 => 184308)
--- trunk/Source/WebCore/editing/Editor.cpp 2015-05-13 21:37:50 UTC (rev 184307)
+++ trunk/Source/WebCore/editing/Editor.cpp 2015-05-13 21:58:41 UTC (rev 184308)
@@ -947,7 +947,7 @@
return;
// FIXME: This is wrong for text decorations since m_mutableStyle is empty.
- if (!client() || !client()->shouldApplyStyle(style->style(), m_frame.selection().toNormalizedRange().get()))
+ if (!client() || !client()->shouldApplyStyle(style->styleWithResolvedTextDecorations().ptr(), m_frame.selection().toNormalizedRange().get()))
return;
applyStyle(WTF::move(style), editingAction);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
