Diff
Modified: branches/safari-601.1.32.2-branch/LayoutTests/ChangeLog (184713 => 184714)
--- branches/safari-601.1.32.2-branch/LayoutTests/ChangeLog 2015-05-21 12:29:43 UTC (rev 184713)
+++ branches/safari-601.1.32.2-branch/LayoutTests/ChangeLog 2015-05-21 15:24:21 UTC (rev 184714)
@@ -1,3 +1,21 @@
+2015-05-20 Babak Shafiei <bshaf...@apple.com>
+
+ Merge r184308.
+
+ 2015-05-13 Ryosuke Niwa <rn...@webkit.org>
+
+ 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-18 Matthew Hanson <matthew_han...@apple.com>
Merge r184510. rdar://problem/21004989
Copied: branches/safari-601.1.32.2-branch/LayoutTests/editing/style/underline-by-user-expected.txt (from rev 184308, trunk/LayoutTests/editing/style/underline-by-user-expected.txt) (0 => 184714)
--- branches/safari-601.1.32.2-branch/LayoutTests/editing/style/underline-by-user-expected.txt (rev 0)
+++ branches/safari-601.1.32.2-branch/LayoutTests/editing/style/underline-by-user-expected.txt 2015-05-21 15:24:21 UTC (rev 184714)
@@ -0,0 +1,3 @@
+This tests user initialized underlining of text. To manually test, underline "hello" below. WebKit should not crash.
+
+hello
Copied: branches/safari-601.1.32.2-branch/LayoutTests/editing/style/underline-by-user.html (from rev 184308, trunk/LayoutTests/editing/style/underline-by-user.html) (0 => 184714)
--- branches/safari-601.1.32.2-branch/LayoutTests/editing/style/underline-by-user.html (rev 0)
+++ branches/safari-601.1.32.2-branch/LayoutTests/editing/style/underline-by-user.html 2015-05-21 15:24:21 UTC (rev 184714)
@@ -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: branches/safari-601.1.32.2-branch/Source/WebCore/ChangeLog (184713 => 184714)
--- branches/safari-601.1.32.2-branch/Source/WebCore/ChangeLog 2015-05-21 12:29:43 UTC (rev 184713)
+++ branches/safari-601.1.32.2-branch/Source/WebCore/ChangeLog 2015-05-21 15:24:21 UTC (rev 184714)
@@ -1,3 +1,34 @@
+2015-05-20 Babak Shafiei <bshaf...@apple.com>
+
+ Merge r184308.
+
+ 2015-05-13 Ryosuke Niwa <rn...@webkit.org>
+
+ 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-20 Matthew Hanson <matthew_han...@apple.com>
Merge r184654. rdar://problem/21044514
Modified: branches/safari-601.1.32.2-branch/Source/WebCore/editing/EditingStyle.cpp (184713 => 184714)
--- branches/safari-601.1.32.2-branch/Source/WebCore/editing/EditingStyle.cpp 2015-05-21 12:29:43 UTC (rev 184713)
+++ branches/safari-601.1.32.2-branch/Source/WebCore/editing/EditingStyle.cpp 2015-05-21 15:24:21 UTC (rev 184714)
@@ -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: branches/safari-601.1.32.2-branch/Source/WebCore/editing/EditingStyle.h (184713 => 184714)
--- branches/safari-601.1.32.2-branch/Source/WebCore/editing/EditingStyle.h 2015-05-21 12:29:43 UTC (rev 184713)
+++ branches/safari-601.1.32.2-branch/Source/WebCore/editing/EditingStyle.h 2015-05-21 15:24:21 UTC (rev 184714)
@@ -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: branches/safari-601.1.32.2-branch/Source/WebCore/editing/Editor.cpp (184713 => 184714)
--- branches/safari-601.1.32.2-branch/Source/WebCore/editing/Editor.cpp 2015-05-21 12:29:43 UTC (rev 184713)
+++ branches/safari-601.1.32.2-branch/Source/WebCore/editing/Editor.cpp 2015-05-21 15:24:21 UTC (rev 184714)
@@ -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);
