Diff
Modified: trunk/LayoutTests/ChangeLog (211022 => 211023)
--- trunk/LayoutTests/ChangeLog 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/LayoutTests/ChangeLog 2017-01-22 05:19:17 UTC (rev 211023)
@@ -1,5 +1,30 @@
2017-01-21 Chris Dumez <[email protected]>
+ innerText should replace existing text node
+ https://bugs.webkit.org/show_bug.cgi?id=167116
+
+ Reviewed by Darin Adler.
+
+ * accessibility/mac/aria-liveregions-changedtext.html:
+ The text is using innerText and changed behavior now that we stopped
+ reusing the existing Text child. Code in RenderObject::willBeDestroyed()
+ is supposed to call AXObjectCache::childrenChanged(parent()) to fire
+ the AXLiveRegionChanged notification. However, it did not because the
+ parent renderer did not have an associated AccessibilityObject.
+
+ * fast/dom/HTMLElement/set-inner-outer-optimization.html:
+ Update existing test which expected the non spec-compliant Text child
+ optimization.
+
+ * fast/repaint/vertical-text-repaint-expected.txt:
+ * fast/repaint/vertical-text-repaint.html:
+ Update / rebaseline test. We now repaint each 80x80 rectangle instead of
+ only repainting the text rects because we remove the Text node then add
+ a new one instead of only updating the existing Text node's test. The
+ output looks exactly the same otherwise.
+
+2017-01-21 Chris Dumez <[email protected]>
+
AccessibilityRenderObject::textChanged() bypasses AXLiveRegionChanged notification coalescing
https://bugs.webkit.org/show_bug.cgi?id=167286
<rdar://problem/30133211>
Modified: trunk/LayoutTests/accessibility/mac/aria-liveregions-changedtext.html (211022 => 211023)
--- trunk/LayoutTests/accessibility/mac/aria-liveregions-changedtext.html 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/LayoutTests/accessibility/mac/aria-liveregions-changedtext.html 2017-01-22 05:19:17 UTC (rev 211023)
@@ -29,8 +29,7 @@
if (window.accessibilityController) {
window.testRunner.waitUntilDone();
- document.getElementById("liveregion").focus();
- liveRegionText = window.accessibilityController.focusedElement;
+ liveRegionText = accessibilityController.accessibleElementById("liveregion");
var addedNotification = liveRegionText.addNotificationListener(ariaCallbackText);
shouldBe("addedNotification", "true");
Modified: trunk/LayoutTests/fast/dom/HTMLElement/set-inner-outer-optimization.html (211022 => 211023)
--- trunk/LayoutTests/fast/dom/HTMLElement/set-inner-outer-optimization.html 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/LayoutTests/fast/dom/HTMLElement/set-inner-outer-optimization.html 2017-01-22 05:19:17 UTC (rev 211023)
@@ -99,8 +99,8 @@
runTest('', 'innerText', '<a></a><b></b>', 'modified');
runTest('text', 'innerText', '', 'modified');
- runTest('text', 'innerText', 'different text', 'modified, with same first child');
- runTest('text', 'innerText', 'text', 'modified, with same first child');
+ runTest('text', 'innerText', 'different text', 'modified');
+ runTest('text', 'innerText', 'text', 'modified');
runTest('<a></a>', 'innerText', '', 'modified');
runTest('<a></a>', 'innerText', 'text', 'modified');
Modified: trunk/LayoutTests/fast/repaint/vertical-text-repaint-expected.txt (211022 => 211023)
--- trunk/LayoutTests/fast/repaint/vertical-text-repaint-expected.txt 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/LayoutTests/fast/repaint/vertical-text-repaint-expected.txt 2017-01-22 05:19:17 UTC (rev 211023)
@@ -1,9 +1,33 @@
-PASS window.internals.repaintRectsAsText().indexOf('95 25') is not -1
-PASS window.internals.repaintRectsAsText().indexOf('95 155') is not -1
-PASS window.internals.repaintRectsAsText().indexOf('95 285') is not -1
-PASS window.internals.repaintRectsAsText().indexOf('225 25') is not -1
-PASS window.internals.repaintRectsAsText().indexOf('225 155') is not -1
-PASS window.internals.repaintRectsAsText().indexOf('225 285') is not -1
+(repaint rects
+ (rect 25 25 80 80)
+ (rect 25 155 80 80)
+ (rect 25 285 80 80)
+ (rect 155 25 80 80)
+ (rect 155 155 80 80)
+ (rect 155 155 80 80)
+ (rect 155 285 80 80)
+ (rect 25 25 80 80)
+ (rect 25 155 80 80)
+ (rect 25 285 80 80)
+ (rect 155 25 80 80)
+ (rect 155 155 80 80)
+ (rect 155 155 80 80)
+ (rect 155 285 80 80)
+ (rect 25 25 80 80)
+ (rect 25 155 80 80)
+ (rect 25 285 80 80)
+ (rect 155 25 80 80)
+ (rect 155 155 80 80)
+ (rect 155 155 80 80)
+ (rect 155 285 80 80)
+)
+
+PASS internals.repaintRectsAsText().indexOf('25 25') is not -1
+PASS internals.repaintRectsAsText().indexOf('25 155') is not -1
+PASS internals.repaintRectsAsText().indexOf('25 285') is not -1
+PASS internals.repaintRectsAsText().indexOf('155 25') is not -1
+PASS internals.repaintRectsAsText().indexOf('155 155') is not -1
+PASS internals.repaintRectsAsText().indexOf('155 285') is not -1
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/fast/repaint/vertical-text-repaint.html (211022 => 211023)
--- trunk/LayoutTests/fast/repaint/vertical-text-repaint.html 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/LayoutTests/fast/repaint/vertical-text-repaint.html 2017-01-22 05:19:17 UTC (rev 211023)
@@ -90,12 +90,13 @@
document.body.offsetWidth;
if (window.internals) {
- shouldNotBe("window.internals.repaintRectsAsText().indexOf('95 25')", "-1");
- shouldNotBe("window.internals.repaintRectsAsText().indexOf('95 155')", "-1");
- shouldNotBe("window.internals.repaintRectsAsText().indexOf('95 285')", "-1");
- shouldNotBe("window.internals.repaintRectsAsText().indexOf('225 25')", "-1");
- shouldNotBe("window.internals.repaintRectsAsText().indexOf('225 155')", "-1");
- shouldNotBe("window.internals.repaintRectsAsText().indexOf('225 285')", "-1");
+ debug(internals.repaintRectsAsText());
+ shouldNotBe("internals.repaintRectsAsText().indexOf('25 25')", "-1");
+ shouldNotBe("internals.repaintRectsAsText().indexOf('25 155')", "-1");
+ shouldNotBe("internals.repaintRectsAsText().indexOf('25 285')", "-1");
+ shouldNotBe("internals.repaintRectsAsText().indexOf('155 25')", "-1");
+ shouldNotBe("internals.repaintRectsAsText().indexOf('155 155')", "-1");
+ shouldNotBe("internals.repaintRectsAsText().indexOf('155 285')", "-1");
internals.stopTrackingRepaints();
}
finishJSTest();
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (211022 => 211023)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2017-01-22 05:19:17 UTC (rev 211023)
@@ -1,3 +1,14 @@
+2017-01-21 Chris Dumez <[email protected]>
+
+ innerText should replace existing text node
+ https://bugs.webkit.org/show_bug.cgi?id=167116
+
+ Reviewed by Darin Adler.
+
+ Rebaseline existing test now that one more check is passing.
+
+ * web-platform-tests/innerText/setter-expected.txt:
+
2017-01-20 Chris Dumez <[email protected]>
Unreviewed, rebaseline html/dom/interfaces.html.
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/innerText/setter-expected.txt (211022 => 211023)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/innerText/setter-expected.txt 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/innerText/setter-expected.txt 2017-01-22 05:19:17 UTC (rev 211023)
@@ -22,7 +22,7 @@
PASS Leading whitespace preserved
PASS Trailing whitespace preserved
PASS Whitespace not compressed
-FAIL Existing text deleted assert_not_equals: Child should be a *new* text node got disallowed value Text node "abc"
+PASS Existing text deleted
PASS Existing <br> deleted
PASS Assigning the empty string
PASS Assigning null
Modified: trunk/Source/WebCore/ChangeLog (211022 => 211023)
--- trunk/Source/WebCore/ChangeLog 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/Source/WebCore/ChangeLog 2017-01-22 05:19:17 UTC (rev 211023)
@@ -1,5 +1,29 @@
2017-01-21 Chris Dumez <[email protected]>
+ innerText should replace existing text node
+ https://bugs.webkit.org/show_bug.cgi?id=167116
+
+ Reviewed by Darin Adler.
+
+ Update setInnerText() to use ContainerNode::replaceAllChildren()
+ instead of replaceChildrenWithText(). replaceAllChildren() is
+ implemented as per specification:
+ - https://html.spec.whatwg.org/multipage/dom.html#the-innertext-idl-attribute
+ - https://dom.spec.whatwg.org/#concept-node-replace-all
+
+ As a result, we now correctly remove existing children before
+ inserting the new one.
+
+ No new tests, updated existing one.
+
+ * editing/markup.cpp:
+ (WebCore::replaceChildrenWithText): Deleted.
+ * editing/markup.h:
+ * html/HTMLElement.cpp:
+ (WebCore::HTMLElement::setInnerText):
+
+2017-01-21 Chris Dumez <[email protected]>
+
AccessibilityRenderObject::textChanged() bypasses AXLiveRegionChanged notification coalescing
https://bugs.webkit.org/show_bug.cgi?id=167286
<rdar://problem/30133211>
Modified: trunk/Source/WebCore/editing/markup.cpp (211022 => 211023)
--- trunk/Source/WebCore/editing/markup.cpp 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/Source/WebCore/editing/markup.cpp 2017-01-22 05:19:17 UTC (rev 211023)
@@ -1017,23 +1017,4 @@
return containerNode->appendChild(fragment);
}
-ExceptionOr<void> replaceChildrenWithText(ContainerNode& container, const String& text)
-{
- Ref<ContainerNode> containerNode(container);
- ChildListMutationScope mutation(containerNode);
-
- if (hasOneTextChild(containerNode)) {
- downcast<Text>(*containerNode->firstChild()).setData(text);
- return { };
- }
-
- auto textNode = Text::create(containerNode->document(), text);
-
- if (hasOneChild(containerNode))
- return containerNode->replaceChild(textNode, *containerNode->firstChild());
-
- containerNode->removeChildren();
- return containerNode->appendChild(textNode);
}
-
-}
Modified: trunk/Source/WebCore/editing/markup.h (211022 => 211023)
--- trunk/Source/WebCore/editing/markup.h 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/Source/WebCore/editing/markup.h 2017-01-22 05:19:17 UTC (rev 211023)
@@ -57,7 +57,6 @@
// These methods are used by HTMLElement & ShadowRoot to replace the children with respected fragment/text.
ExceptionOr<void> replaceChildrenWithFragment(ContainerNode&, Ref<DocumentFragment>&&);
-ExceptionOr<void> replaceChildrenWithText(ContainerNode&, const String&);
String createMarkup(const Range&, Vector<Node*>* = nullptr, EAnnotateForInterchange = DoNotAnnotateForInterchange, bool convertBlocksToInlines = false, EAbsoluteURLs = DoNotResolveURLs);
String createMarkup(const Node&, EChildrenOnly = IncludeNode, Vector<Node*>* = nullptr, EAbsoluteURLs = DoNotResolveURLs, Vector<QualifiedName>* tagNamesToSkip = nullptr, EFragmentSerialization = HTMLFragmentSerialization);
Modified: trunk/Source/WebCore/html/HTMLElement.cpp (211022 => 211023)
--- trunk/Source/WebCore/html/HTMLElement.cpp 2017-01-22 02:29:15 UTC (rev 211022)
+++ trunk/Source/WebCore/html/HTMLElement.cpp 2017-01-22 05:19:17 UTC (rev 211023)
@@ -503,24 +503,27 @@
// FIXME: This doesn't take whitespace collapsing into account at all.
if (!text.contains('\n') && !text.contains('\r')) {
- if (text.isEmpty()) {
- removeChildren();
- return { };
- }
- return replaceChildrenWithText(*this, text);
+ if (text.isEmpty())
+ replaceAllChildren(nullptr);
+ else
+ replaceAllChildren(document().createTextNode(text));
+ return { };
}
// FIXME: Do we need to be able to detect preserveNewline style even when there's no renderer?
// FIXME: Can the renderer be out of date here? Do we need to call updateStyleIfNeeded?
// For example, for the contents of textarea elements that are display:none?
- auto r = renderer();
+ auto* r = renderer();
if ((r && r->style().preserveNewline()) || (inDocument() && isTextControlInnerTextElement())) {
- if (!text.contains('\r'))
- return replaceChildrenWithText(*this, text);
+ if (!text.contains('\r')) {
+ replaceAllChildren(document().createTextNode(text));
+ return { };
+ }
String textWithConsistentLineBreaks = text;
textWithConsistentLineBreaks.replace("\r\n", "\n");
textWithConsistentLineBreaks.replace('\r', '\n');
- return replaceChildrenWithText(*this, textWithConsistentLineBreaks);
+ replaceAllChildren(document().createTextNode(textWithConsistentLineBreaks));
+ return { };
}
// Add text nodes and <br> elements.
@@ -527,6 +530,7 @@
auto fragment = textToFragment(text);
if (fragment.hasException())
return fragment.releaseException();
+ // FIXME: This should use replaceAllChildren() once it accepts DocumentFragments as input.
return replaceChildrenWithFragment(*this, fragment.releaseReturnValue());
}
