- Revision
- 224649
- Author
- wenson_hs...@apple.com
- Date
- 2017-11-09 15:26:55 -0800 (Thu, 09 Nov 2017)
Log Message
Inserting an image, selecting, underlining, and then deleting leaves the typing style with both "-webkit-text-decorations-in-effect" and "text-decoration"
https://bugs.webkit.org/show_bug.cgi?id=179431
Reviewed by Ryosuke Niwa.
Source/WebCore:
When inserting an image element, selecting it, underlining the selection, deleting, and then inserting text, we
crash on a debug assert. This codepath was exercised by an API test added in <https://trac.webkit.org/r224512>.
This assertion happens due to the following sequence of events:
1. DeleteSelectionCommand::saveTypingStyleState computes a typing style.
2. In doing so, it calls into EditingStyle::init, which observes that "-webkit-text-decorations-in-effect" is
present and appends "text-decoration" with an identical CSS value to the EditingStyle's mutable style
properties.
3. DeleteSelectionCommand::calculateTypingStyleAfterDelete sets the current selection's typing style to the
above typing style.
4. Later on, when we try to insert text, we compute the StyleChange using the above typing style, which calls
into reconcileTextDecorationProperties.
5. reconcileTextDecorationProperties debug asserts that "-webkit-text-decorations-in-effect" and
"text-decoration" don't coexist on the EditingStyle's (i.e. the typing style's) mutable properties; since (2)
added both properties, this assertion fires.
It appears that step (2) shouldn't be adding "text-decoration" in addition to EditingStyle's mutable style
properties, since doing so would violate the requirements of reconcileTextDecorationProperties. As such, we can
tweak EditingStyle::init to *replace* the "-webkit-text-decorations-in-effect" property with "text-decoration"
instead; this matches the behavior of reconcileTextDecorationProperties, and ensures that we only have the
"text-decorations" property when we try to insert text in step (4).
Test: editing/execCommand/underline-selection-containing-image.html
* editing/EditingStyle.cpp:
(WebCore::EditingStyle::init):
LayoutTests:
Adds a new layout test to fix a debug assertion. See WebCore/ChangeLog for more details. Additionally
rebaselines a few existing tests that serialize markup strings to include `text-decoration: none;`.
* editing/execCommand/underline-selection-containing-image-expected.txt: Added.
* editing/execCommand/underline-selection-containing-image.html: Added.
* editing/pasteboard/data-transfer-get-data-on-drop-rich-text-expected.txt:
* fast/events/before-input-events-prevent-drag-and-drop-expected.txt:
* fast/events/input-events-paste-rich-datatransfer-expected.txt:
* fast/events/ondrop-text-html-expected.txt:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (224648 => 224649)
--- trunk/LayoutTests/ChangeLog 2017-11-09 23:12:01 UTC (rev 224648)
+++ trunk/LayoutTests/ChangeLog 2017-11-09 23:26:55 UTC (rev 224649)
@@ -1,3 +1,20 @@
+2017-11-09 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Inserting an image, selecting, underlining, and then deleting leaves the typing style with both "-webkit-text-decorations-in-effect" and "text-decoration"
+ https://bugs.webkit.org/show_bug.cgi?id=179431
+
+ Reviewed by Ryosuke Niwa.
+
+ Adds a new layout test to fix a debug assertion. See WebCore/ChangeLog for more details. Additionally
+ rebaselines a few existing tests that serialize markup strings to include `text-decoration: none;`.
+
+ * editing/execCommand/underline-selection-containing-image-expected.txt: Added.
+ * editing/execCommand/underline-selection-containing-image.html: Added.
+ * editing/pasteboard/data-transfer-get-data-on-drop-rich-text-expected.txt:
+ * fast/events/before-input-events-prevent-drag-and-drop-expected.txt:
+ * fast/events/input-events-paste-rich-datatransfer-expected.txt:
+ * fast/events/ondrop-text-html-expected.txt:
+
2017-11-09 Devin Rousso <web...@devinrousso.com>
Web Inspector: support undo/redo of insertAdjacentHTML
Added: trunk/LayoutTests/editing/execCommand/underline-selection-containing-image-expected.txt (0 => 224649)
--- trunk/LayoutTests/editing/execCommand/underline-selection-containing-image-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/execCommand/underline-selection-containing-image-expected.txt 2017-11-09 23:26:55 UTC (rev 224649)
@@ -0,0 +1 @@
+The underlined text should be 'foo': 'foo'
Added: trunk/LayoutTests/editing/execCommand/underline-selection-containing-image.html (0 => 224649)
--- trunk/LayoutTests/editing/execCommand/underline-selection-containing-image.html (rev 0)
+++ trunk/LayoutTests/editing/execCommand/underline-selection-containing-image.html 2017-11-09 23:26:55 UTC (rev 224649)
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<body contenteditable></body>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+document.body.focus();
+document.execCommand("InsertHTML", true, "<img></img>");
+document.execCommand("SelectAll");
+document.execCommand("Underline");
+document.execCommand("InsertText", true, "foo");
+document.body.textContent = `The underlined text should be 'foo': '${document.querySelector("U").textContent}'`;
+</script>
Modified: trunk/LayoutTests/editing/pasteboard/data-transfer-get-data-on-drop-rich-text-expected.txt (224648 => 224649)
--- trunk/LayoutTests/editing/pasteboard/data-transfer-get-data-on-drop-rich-text-expected.txt 2017-11-09 23:12:01 UTC (rev 224648)
+++ trunk/LayoutTests/editing/pasteboard/data-transfer-get-data-on-drop-rich-text-expected.txt 2017-11-09 23:26:55 UTC (rev 224649)
@@ -5,7 +5,7 @@
"text/plain": ""
},
"drop": {
- "text/html": "<!DOCTYPE html><strong style=\"font-family: -apple-system; font-size: 150px; font-style: normal; font-variant-caps: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: nowrap; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; color: purple;\">Rich text</strong>",
+ "text/html": "<!DOCTYPE html><strong style=\"font-family: -apple-system; font-size: 150px; font-style: normal; font-variant-caps: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: nowrap; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none; color: purple;\">Rich text</strong>",
"text/plain": "Rich text"
}
}
Modified: trunk/LayoutTests/fast/events/before-input-events-prevent-drag-and-drop-expected.txt (224648 => 224649)
--- trunk/LayoutTests/fast/events/before-input-events-prevent-drag-and-drop-expected.txt 2017-11-09 23:12:01 UTC (rev 224648)
+++ trunk/LayoutTests/fast/events/before-input-events-prevent-drag-and-drop-expected.txt 2017-11-09 23:26:55 UTC (rev 224649)
@@ -7,4 +7,4 @@
HTML content:
-<span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: monospace; font-size: 108px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: center; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; display: inline !important; float: none;">input events</span>
+<span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: monospace; font-size: 108px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: center; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none; display: inline !important; float: none;">input events</span>
Modified: trunk/LayoutTests/fast/events/input-events-paste-rich-datatransfer-expected.txt (224648 => 224649)
--- trunk/LayoutTests/fast/events/input-events-paste-rich-datatransfer-expected.txt 2017-11-09 23:12:01 UTC (rev 224648)
+++ trunk/LayoutTests/fast/events/input-events-paste-rich-datatransfer-expected.txt 2017-11-09 23:26:55 UTC (rev 224649)
@@ -2,7 +2,7 @@
destination after pasting (text/html):
| <b>
-| style="caret-color: rgb(255, 0, 0); color: rgb(255, 0, 0); font-family: -webkit-standard; font-style: normal; font-variant-caps: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;"
+| style="caret-color: rgb(255, 0, 0); color: rgb(255, 0, 0); font-family: -webkit-standard; font-style: normal; font-variant-caps: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none;"
| "LayoutTests"
| <i>
| "are"
Modified: trunk/LayoutTests/fast/events/ondrop-text-html-expected.txt (224648 => 224649)
--- trunk/LayoutTests/fast/events/ondrop-text-html-expected.txt 2017-11-09 23:12:01 UTC (rev 224648)
+++ trunk/LayoutTests/fast/events/ondrop-text-html-expected.txt 2017-11-09 23:26:55 UTC (rev 224649)
@@ -1,4 +1,4 @@
CONSOLE MESSAGE: line 21: text/plain: This test verifies that we can get text/html from the drag object during an ondrop event.
-CONSOLE MESSAGE: line 23: text/html: <span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-size: medium; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; display: inline !important; float: none;">This test verifies that we can get text/html from the drag object during an ondrop event.<span class="Apple-converted-space"> </span></span>
+CONSOLE MESSAGE: line 23: text/html: <span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-size: medium; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none; display: inline !important; float: none;">This test verifies that we can get text/html from the drag object during an ondrop event.<span class="Apple-converted-space"> </span></span>
This test verifies that we can get text/html from the drag object during an ondrop event. This test requires DRT.
PASS
Modified: trunk/Source/WebCore/ChangeLog (224648 => 224649)
--- trunk/Source/WebCore/ChangeLog 2017-11-09 23:12:01 UTC (rev 224648)
+++ trunk/Source/WebCore/ChangeLog 2017-11-09 23:26:55 UTC (rev 224649)
@@ -1,3 +1,36 @@
+2017-11-09 Wenson Hsieh <wenson_hs...@apple.com>
+
+ Inserting an image, selecting, underlining, and then deleting leaves the typing style with both "-webkit-text-decorations-in-effect" and "text-decoration"
+ https://bugs.webkit.org/show_bug.cgi?id=179431
+
+ Reviewed by Ryosuke Niwa.
+
+ When inserting an image element, selecting it, underlining the selection, deleting, and then inserting text, we
+ crash on a debug assert. This codepath was exercised by an API test added in <https://trac.webkit.org/r224512>.
+ This assertion happens due to the following sequence of events:
+ 1. DeleteSelectionCommand::saveTypingStyleState computes a typing style.
+ 2. In doing so, it calls into EditingStyle::init, which observes that "-webkit-text-decorations-in-effect" is
+ present and appends "text-decoration" with an identical CSS value to the EditingStyle's mutable style
+ properties.
+ 3. DeleteSelectionCommand::calculateTypingStyleAfterDelete sets the current selection's typing style to the
+ above typing style.
+ 4. Later on, when we try to insert text, we compute the StyleChange using the above typing style, which calls
+ into reconcileTextDecorationProperties.
+ 5. reconcileTextDecorationProperties debug asserts that "-webkit-text-decorations-in-effect" and
+ "text-decoration" don't coexist on the EditingStyle's (i.e. the typing style's) mutable properties; since (2)
+ added both properties, this assertion fires.
+
+ It appears that step (2) shouldn't be adding "text-decoration" in addition to EditingStyle's mutable style
+ properties, since doing so would violate the requirements of reconcileTextDecorationProperties. As such, we can
+ tweak EditingStyle::init to *replace* the "-webkit-text-decorations-in-effect" property with "text-decoration"
+ instead; this matches the behavior of reconcileTextDecorationProperties, and ensures that we only have the
+ "text-decorations" property when we try to insert text in step (4).
+
+ Test: editing/execCommand/underline-selection-containing-image.html
+
+ * editing/EditingStyle.cpp:
+ (WebCore::EditingStyle::init):
+
2017-11-09 Devin Rousso <web...@devinrousso.com>
Web Inspector: support undo/redo of insertAdjacentHTML
Modified: trunk/Source/WebCore/editing/EditingStyle.cpp (224648 => 224649)
--- trunk/Source/WebCore/editing/EditingStyle.cpp 2017-11-09 23:12:01 UTC (rev 224648)
+++ trunk/Source/WebCore/editing/EditingStyle.cpp 2017-11-09 23:26:55 UTC (rev 224649)
@@ -455,8 +455,10 @@
if (propertiesToInclude == EditingPropertiesInEffect) {
if (RefPtr<CSSValue> value = backgroundColorInEffect(node))
m_mutableStyle->setProperty(CSSPropertyBackgroundColor, value->cssText());
- if (RefPtr<CSSValue> value = computedStyleAtPosition.propertyValue(CSSPropertyWebkitTextDecorationsInEffect))
+ if (RefPtr<CSSValue> value = computedStyleAtPosition.propertyValue(CSSPropertyWebkitTextDecorationsInEffect)) {
m_mutableStyle->setProperty(CSSPropertyTextDecoration, value->cssText());
+ m_mutableStyle->removeProperty(CSSPropertyWebkitTextDecorationsInEffect);
+ }
}
if (node && node->computedStyle()) {
