Diff
Modified: trunk/LayoutTests/ChangeLog (245911 => 245912)
--- trunk/LayoutTests/ChangeLog 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/LayoutTests/ChangeLog 2019-05-31 00:02:59 UTC (rev 245912)
@@ -1,3 +1,18 @@
+2019-05-30 Andres Gonzalez <[email protected]>
+
+ Inserting a newline in contenteditable causes two characters to be added instead of one
+ https://bugs.webkit.org/show_bug.cgi?id=197894
+ <rdar://problem/49700998>
+
+ Reviewed by Wenson Hsieh and Chris Fleizach.
+
+ * accessibility/ios-simulator/set-selected-text-range-after-newline-expected.txt: Added.
+ * accessibility/ios-simulator/set-selected-text-range-after-newline.html: Added.
+ * accessibility/ios-simulator/text-marker-list-item-expected.txt:
+ * accessibility/set-selected-text-range-after-newline-expected.txt: Added.
+ * accessibility/set-selected-text-range-after-newline.html: Added.
+ * platform/win/TestExpectations:
+
2019-05-30 Devin Rousso <[email protected]>
Web Inspector: Audit: tests are unable to get the current Audit version
Added: trunk/LayoutTests/accessibility/ios-simulator/set-selected-text-range-after-newline-expected.txt (0 => 245912)
--- trunk/LayoutTests/accessibility/ios-simulator/set-selected-text-range-after-newline-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/ios-simulator/set-selected-text-range-after-newline-expected.txt 2019-05-31 00:02:59 UTC (rev 245912)
@@ -0,0 +1,10 @@
+hello
+world
+PASS text.selectedTextRange became '{5, 0}'
+There must be only one [newline] between hello and world: hello[newline]world
+PASS text.selectedTextRange became '{6, 0}'
+The text after the newline should be world: world
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/accessibility/ios-simulator/set-selected-text-range-after-newline.html (0 => 245912)
--- trunk/LayoutTests/accessibility/ios-simulator/set-selected-text-range-after-newline.html (rev 0)
+++ trunk/LayoutTests/accessibility/ios-simulator/set-selected-text-range-after-newline.html 2019-05-31 00:02:59 UTC (rev 245912)
@@ -0,0 +1,40 @@
+<html>
+<head>
+<script src=""
+</head>
+<body>
+
+<div id="content" contenteditable tabindex="0">helloworld</div>
+
+<div id="console"></div>
+
+<script>
+ if (window.accessibilityController) {
+ window.jsTestIsAsync = true;
+
+ var content = document.getElementById("content");
+ content.focus();
+
+ var text = accessibilityController.focusedElement;
+ text.setSelectedTextRange(5, 0);
+ shouldBecomeEqual("text.selectedTextRange", "'{5, 0}'", function() {
+ text.replaceTextInRange("\n", 5, 0);
+
+ var t = text.stringForRange(0, 11);
+ t = t.replace(/(?:\r\n|\r|\n)/g, '[newline]');
+ debug("There must be only one [newline] between hello and world: " + t);
+
+ text.setSelectedTextRange(6, 0);
+ shouldBecomeEqual("text.selectedTextRange", "'{6, 0}'", function() {
+ var t = text.stringForRange(6, 5);
+ t = t.replace(/(?:\r\n|\r|\n)/g, '[newline]');
+ debug("The text after the newline should be world: " + t);
+
+ finishJSTest();
+ });
+ });
+ }
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/accessibility/ios-simulator/text-marker-list-item-expected.txt (245911 => 245912)
--- trunk/LayoutTests/accessibility/ios-simulator/text-marker-list-item-expected.txt 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/LayoutTests/accessibility/ios-simulator/text-marker-list-item-expected.txt 2019-05-31 00:02:59 UTC (rev 245912)
@@ -4,7 +4,7 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-FAIL text should be 1. item 1. Was 1. .
+PASS text is '1. item 1'
PASS successfullyParsed is true
TEST COMPLETE
Added: trunk/LayoutTests/accessibility/set-selected-text-range-after-newline-expected.txt (0 => 245912)
--- trunk/LayoutTests/accessibility/set-selected-text-range-after-newline-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/set-selected-text-range-after-newline-expected.txt 2019-05-31 00:02:59 UTC (rev 245912)
@@ -0,0 +1,10 @@
+hello
+world
+PASS text.selectedTextRange became '{5, 0}'
+There must be only one [newline] between hello and world: hello[newline]world
+PASS text.selectedTextRange became '{6, 0}'
+The text after the newline should be world: world
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/accessibility/set-selected-text-range-after-newline.html (0 => 245912)
--- trunk/LayoutTests/accessibility/set-selected-text-range-after-newline.html (rev 0)
+++ trunk/LayoutTests/accessibility/set-selected-text-range-after-newline.html 2019-05-31 00:02:59 UTC (rev 245912)
@@ -0,0 +1,40 @@
+<html>
+<head>
+<script src=""
+</head>
+<body>
+
+<div id="content" contenteditable tabindex="0">helloworld</div>
+
+<div id="console"></div>
+
+<script>
+ if (window.accessibilityController) {
+ window.jsTestIsAsync = true;
+
+ var content = document.getElementById("content");
+ content.focus();
+
+ var text = accessibilityController.focusedElement;
+ text.setSelectedTextRange(5, 0);
+ shouldBecomeEqual("text.selectedTextRange", "'{5, 0}'", function() {
+ text.replaceTextInRange("\n", 5, 0);
+
+ var t = text.stringForRange(0, 11);
+ t = t.replace(/(?:\r\n|\r|\n)/g, '[newline]');
+ debug("There must be only one [newline] between hello and world: " + t);
+
+ text.setSelectedTextRange(6, 0);
+ shouldBecomeEqual("text.selectedTextRange", "'{6, 0}'", function() {
+ var t = text.stringForRange(6, 5);
+ t = t.replace(/(?:\r\n|\r|\n)/g, '[newline]');
+ debug("The text after the newline should be world: " + t);
+
+ finishJSTest();
+ });
+ });
+ }
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/platform/win/TestExpectations (245911 => 245912)
--- trunk/LayoutTests/platform/win/TestExpectations 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/LayoutTests/platform/win/TestExpectations 2019-05-31 00:02:59 UTC (rev 245912)
@@ -3394,6 +3394,7 @@
js/dom/create-lots-of-workers.html [ Crash ]
# Timeouts tracked in webkit.org/b/160447.
accessibility/set-selected-text-range-contenteditable.html [ Skip ]
+accessibility/set-selected-text-range-after-newline.html [ Skip ]
crypto/crypto-key-algorithm-gc.html [ Skip ]
crypto/crypto-key-usages-gc.html [ Skip ]
editing/deleting/delete-emoji.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (245911 => 245912)
--- trunk/Source/WebCore/ChangeLog 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/Source/WebCore/ChangeLog 2019-05-31 00:02:59 UTC (rev 245912)
@@ -1,3 +1,41 @@
+2019-05-30 Andres Gonzalez <[email protected]>
+
+ Inserting a newline in contenteditable causes two characters to be added instead of one
+ https://bugs.webkit.org/show_bug.cgi?id=197894
+ <rdar://problem/49700998>
+
+ Reviewed by Wenson Hsieh and Chris Fleizach.
+
+ There were two issues with inserting a newline character at the end of
+ a line that caused problems for accessibility:
+ - the first '\n' inserted after text would result in two line breaks
+ inserted instead of one. createFragmentFromText in markup.cpp was
+ splitting the string "\n" into two empty strings and creating a <div>
+ and a <br> respectively. Then the emission code would emit a '\n' for
+ the empty div and another for the <br>.
+ - the second problem is a consequence of <rdar://problem/5192593> and
+ the workaround is the change in editing.cpp in the function
+ visiblePositionForIndexUsingCharacterIterator, similar to what is done
+ in VisibleUnits.cpp for nextBoundary.
+ The rest of the changes in this patch are accessibility changes to
+ execute the layout tests.
+
+ Tests: accessibility/ios-simulator/set-selected-text-range-after-newline.html
+ accessibility/set-selected-text-range-after-newline.html
+
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::setSelectedTextRange):
+ * accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
+ (-[WebAccessibilityObjectWrapper stringForRange:]):
+ (-[WebAccessibilityObjectWrapper _accessibilitySelectedTextRange]):
+ (-[WebAccessibilityObjectWrapper accessibilityReplaceRange:withText:]):
+ * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+ (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
+ * editing/Editing.cpp:
+ (WebCore::visiblePositionForIndexUsingCharacterIterator):
+ * editing/markup.cpp:
+ (WebCore::createFragmentFromText):
+
2019-05-30 Justin Fan <[email protected]>
[Web GPU] Vertex Buffers/Input State API updates
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (245911 => 245912)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2019-05-31 00:02:59 UTC (rev 245912)
@@ -1621,9 +1621,10 @@
HTMLTextFormControlElement& textControl = downcast<RenderTextControl>(*m_renderer).textFormControlElement();
textControl.setSelectionRange(range.start, range.start + range.length);
} else {
- ASSERT(node());
- VisiblePosition start = visiblePositionForIndexUsingCharacterIterator(*node(), range.start);
- VisiblePosition end = visiblePositionForIndexUsingCharacterIterator(*node(), range.start + range.length);
+ auto node = this->node();
+ ASSERT(node);
+ VisiblePosition start = visiblePositionForIndexUsingCharacterIterator(*node, range.start);
+ VisiblePosition end = visiblePositionForIndexUsingCharacterIterator(*node, range.start + range.length);
m_renderer->frame().selection().setSelection(VisibleSelection(start, end), FrameSelection::defaultSetSelectionOptions(UserTriggered));
}
Modified: trunk/Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm (245911 => 245912)
--- trunk/Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm 2019-05-31 00:02:59 UTC (rev 245912)
@@ -2497,11 +2497,13 @@
return [self _stringFromStartMarker:startMarker toEndMarker:endMarker attributed:attributed];
}
-
-// A convenience method for getting the text of a NSRange. Currently used only by DRT.
+// A convenience method for getting the text of a NSRange.
- (NSString *)stringForRange:(NSRange)range
{
- return [self _stringForRange:range attributed:NO];
+ if (![self _prepareAccessibilityCall])
+ return nil;
+
+ return m_object->stringForRange([self _convertToDOMRange:range]);
}
- (NSAttributedString *)attributedStringForRange:(NSRange)range
@@ -2525,11 +2527,11 @@
{
if (![self _prepareAccessibilityCall] || !m_object->isTextControl())
return NSMakeRange(NSNotFound, 0);
-
+
PlainTextRange textRange = m_object->selectedTextRange();
if (textRange.isNull())
return NSMakeRange(NSNotFound, 0);
- return NSMakeRange(textRange.start, textRange.length);
+ return NSMakeRange(textRange.start, textRange.length);
}
- (void)_accessibilitySetSelectedTextRange:(NSRange)range
@@ -2540,6 +2542,14 @@
m_object->setSelectedTextRange(PlainTextRange(range.location, range.length));
}
+- (BOOL)accessibilityReplaceRange:(NSRange)range withText:(NSString *)string
+{
+ if (![self _prepareAccessibilityCall])
+ return NO;
+
+ return m_object->replaceTextInRange(string, PlainTextRange(range));
+}
+
// A convenience method for getting the accessibility objects of a NSRange. Currently used only by DRT.
- (NSArray *)elementsForRange:(NSRange)range
{
Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm (245911 => 245912)
--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm 2019-05-31 00:02:59 UTC (rev 245912)
@@ -2742,8 +2742,6 @@
}
if ([attributeName isEqualToString: NSAccessibilitySelectedTextRangeAttribute]) {
PlainTextRange textRange = m_object->selectedTextRange();
- if (textRange.isNull())
- return [NSValue valueWithRange:NSMakeRange(0, 0)];
return [NSValue valueWithRange:NSMakeRange(textRange.start, textRange.length)];
}
// TODO: Get actual visible range. <rdar://problem/4712101>
Modified: trunk/Source/WebCore/editing/Editing.cpp (245911 => 245912)
--- trunk/Source/WebCore/editing/Editing.cpp 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/Source/WebCore/editing/Editing.cpp 2019-05-31 00:02:59 UTC (rev 245912)
@@ -1121,6 +1121,16 @@
range->selectNodeContents(node);
CharacterIterator it(range.get());
it.advance(index - 1);
+
+ if (!it.atEnd() && it.text()[0] == '\n') {
+ // FIXME: workaround for collapsed range (where only start position is correct) emitted for some emitted newlines (see rdar://5192593)
+ auto range = it.range();
+ if (range->startPosition() == range->endPosition()) {
+ it.advance(1);
+ return VisiblePosition(it.range()->startPosition());
+ }
+ }
+
return { it.atEnd() ? range->endPosition() : it.range()->endPosition(), UPSTREAM };
}
Modified: trunk/Source/WebCore/editing/markup.cpp (245911 => 245912)
--- trunk/Source/WebCore/editing/markup.cpp 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/Source/WebCore/editing/markup.cpp 2019-05-31 00:02:59 UTC (rev 245912)
@@ -1115,12 +1115,16 @@
string.replace("\r\n", "\n");
string.replace('\r', '\n');
+ auto createHTMLBRElement = [&document]() {
+ auto element = HTMLBRElement::create(document);
+ element->setAttributeWithoutSynchronization(classAttr, AppleInterchangeNewline);
+ return element;
+ };
+
if (contextPreservesNewline(context)) {
fragment->appendChild(document.createTextNode(string));
if (string.endsWith('\n')) {
- auto element = HTMLBRElement::create(document);
- element->setAttributeWithoutSynchronization(classAttr, AppleInterchangeNewline);
- fragment->appendChild(element);
+ fragment->appendChild(createHTMLBRElement());
}
return fragment;
}
@@ -1131,6 +1135,12 @@
return fragment;
}
+ if (string.length() == 1 && string[0] == '\n') {
+ // This is a single newline char, thus just create one HTMLBRElement.
+ fragment->appendChild(createHTMLBRElement());
+ return fragment;
+ }
+
// Break string into paragraphs. Extra line breaks turn into empty paragraphs.
Node* blockNode = enclosingBlock(context.firstNode());
Element* block = downcast<Element>(blockNode);
@@ -1149,8 +1159,7 @@
RefPtr<Element> element;
if (s.isEmpty() && i + 1 == numLines) {
// For last line, use the "magic BR" rather than a P.
- element = HTMLBRElement::create(document);
- element->setAttributeWithoutSynchronization(classAttr, AppleInterchangeNewline);
+ element = createHTMLBRElement();
} else if (useLineBreak) {
element = HTMLBRElement::create(document);
fillContainerFromString(fragment, s);
Modified: trunk/Tools/ChangeLog (245911 => 245912)
--- trunk/Tools/ChangeLog 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/Tools/ChangeLog 2019-05-31 00:02:59 UTC (rev 245912)
@@ -1,3 +1,19 @@
+2019-05-30 Andres Gonzalez <[email protected]>
+
+ Inserting a newline in contenteditable causes two characters to be added instead of one
+ https://bugs.webkit.org/show_bug.cgi?id=197894
+ <rdar://problem/49700998>
+
+ Reviewed by Wenson Hsieh and Chris Fleizach.
+
+ iOS implementation of several AccessibilityUIElement methods to execute
+ LayoutTests.
+
+ * WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:
+ (WTR::AccessibilityUIElement::selectedTextRange):
+ (WTR::AccessibilityUIElement::setSelectedTextRange):
+ (WTR::AccessibilityUIElement::replaceTextInRange):
+
2019-05-30 Keith Miller <[email protected]>
IsoHeaps don't notice uncommitted VA becoming the first eligible.
Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm (245911 => 245912)
--- trunk/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm 2019-05-31 00:00:09 UTC (rev 245911)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm 2019-05-31 00:02:59 UTC (rev 245912)
@@ -55,6 +55,9 @@
- (NSString *)selectionRangeString;
- (CGPoint)accessibilityClickPoint;
- (void)accessibilityModifySelection:(WebCore::TextGranularity)granularity increase:(BOOL)increase;
+- (NSRange)_accessibilitySelectedTextRange;
+- (void)_accessibilitySetSelectedTextRange:(NSRange)range;
+- (BOOL)accessibilityReplaceRange:(NSRange)range withText:(NSString *)string;
- (void)accessibilitySetPostedNotificationCallback:(AXPostedNotificationCallback)function withContext:(void*)context;
- (CGFloat)_accessibilityMinValue;
- (CGFloat)_accessibilityMaxValue;
@@ -836,7 +839,9 @@
JSRetainPtr<JSStringRef> AccessibilityUIElement::selectedTextRange()
{
- return createEmptyJSString();
+ NSRange range = [m_element _accessibilitySelectedTextRange];
+ NSMutableString *rangeDescription = [NSMutableString stringWithFormat:@"{%lu, %lu}", static_cast<unsigned long>(range.location), static_cast<unsigned long>(range.length)];
+ return [rangeDescription createJSStringRef];
}
bool AccessibilityUIElement::setSelectedVisibleTextRange(AccessibilityTextMarkerRange*)
@@ -846,7 +851,8 @@
bool AccessibilityUIElement::setSelectedTextRange(unsigned location, unsigned length)
{
- return false;
+ [m_element _accessibilitySetSelectedTextRange:NSMakeRange(location, length)];
+ return true;
}
void AccessibilityUIElement::increment()
@@ -1145,9 +1151,9 @@
return nullptr;
}
-bool AccessibilityUIElement::replaceTextInRange(JSStringRef, int, int)
+bool AccessibilityUIElement::replaceTextInRange(JSStringRef string, int location, int length)
{
- return false;
+ return [m_element accessibilityReplaceRange:NSMakeRange(location, length) withText:[NSString stringWithJSStringRef:string]];
}
RefPtr<AccessibilityTextMarker> AccessibilityUIElement::textMarkerForPoint(int x, int y)