Bug 194880 – Smart Insert for paragraphs.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Megan Gardner]

Smart Insert for paragraphs.

[Megan Gardner]

Created attachment 362561 [details]
Patch

[EWS Watchlist]

Attachment 362561 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 362561 [details]
Patch

Attachment 362561 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11225287

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 362570 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 362561 [details]
Patch

Attachment 362561 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11225445

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 362571 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 362561 [details]
Patch

Attachment 362561 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11225334

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 362575 [details]
Archive of layout-test-results from ews113 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Ryosuke Niwa]

Comment on attachment 362561 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362561&action=review

> Source/WebCore/editing/CompositeEditCommand.h:155
> +    void insertParagraphSeparatorAtPosition(const Position&, bool useDefaultParagraphElement = false, bool pasteBlockqutoeIntoUnquotedArea = false);

We don't want to do this. We want to update the selection & have insertParagraphSeparator adjust the selection as it mutates DOM.
Otherwise, we can end up with an orphaned position (i.e. position is no long in document).

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1273
> +        addNewLinesForSmartReplace();

We definitely want to have a separate check for this feature.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1348
> +    Node* endNode = endUpstream.computeNodeBeforePosition();
> +    int endOffset = is<Text>(endNode) ? downcast<Text>(*endNode).length() : 0;
> +    if (endUpstream.anchorType() == Position::PositionIsOffsetInAnchor) {
> +        endNode = endUpstream.containerNode();
> +        endOffset = endUpstream.offsetInContainerNode();
> +    }

This doesn't look right. Why are getting these node & offset?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1356
> +    Position startDownstream = startOfInsertedContent.deepEquivalent().downstream();
> +    Node* startNode = startDownstream.computeNodeAfterPosition();
> +    unsigned startOffset = 0;
> +    if (startDownstream.anchorType() == Position::PositionIsOffsetInAnchor) {
> +        startNode = startDownstream.containerNode();
> +        startOffset = startDownstream.offsetInContainerNode();
> +    }

Ditto.

[EWS Watchlist]

Comment on attachment 362561 [details]
Patch

Attachment 362561 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/11226425

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 362581 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Ryosuke Niwa]

Comment on attachment 362561 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362561&action=review

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1366
> +        if (isEndOfLine(positionBeforeStart))

Doesn't this return true even when positionBeforeStart is on an empty line?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1373
> +        if (isStartOfLine(positionAfterEnd))

Ditto.

[Megan Gardner]

Created attachment 362752 [details]
Patch

[EWS Watchlist]

Attachment 362752 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Comment on attachment 362752 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362752&action=review

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1272
> +    

Nit: whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1275
> +    

Ditto.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1337
> +    // <MMG> add a check for iOS

You mean IMG?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1338
> +    

Nit: whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1355
> +    

Nit: Whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1358
> +    Position endUpstream = endOfInsertedContent.deepEquivalent().upstream();
> +    
> +    Position startDownstream = startOfInsertedContent.deepEquivalent().downstream();

These things aren't used anywhere. Remove?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1359
> +    

Nit: whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1361
> +    

Ditto.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1364
> +    

Ditto.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1369
> +            applyCommandToComposite(SetSelectionCommand::create(VisibleSelection(startOfInsertedContent), FrameSelection::defaultSetSelectionOptions()));

Just call setEndingSelection instead.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1378
> +            applyCommandToComposite(SetSelectionCommand::create(VisibleSelection(endOfInsertedContent), FrameSelection::defaultSetSelectionOptions()));

Ditto.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1382
> +}

We need to restore endingSelection here in the case we've inserted paragraphs above.

[Megan Gardner]

Created attachment 362948 [details]
Patch

[Megan Gardner]

Created attachment 363023 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 363023 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363023&action=review

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1340
> +    Element* textControl = enclosingTextFormControl(positionAtStartOfInsertedContent().deepEquivalent());

Do we really need this check? Also use auto?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1359
> +    Position endUpstream = endOfInsertedContent.deepEquivalent().upstream();
> +    
> +    Position startDownstream = startOfInsertedContent.deepEquivalent().downstream();

These two variables are never used. Remove?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1372
> +            setEndingSelection(VisibleSelection(startOfInsertedContent));

This is correct use of setEndingSelection but the conversion to VisibleSelection is implicit
so you can just do setEndingSelection(startOfInsertedContent).

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1382
> +            setEndingSelection(VisibleSelection(endOfInsertedContent));

Ditto.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1388
> +    }

As I commented in the previous patch, we probably need to restore ending selection here in the case we've modified above.

[Megan Gardner]

Created attachment 363050 [details]
Patch

[Megan Gardner]

(In reply to Ryosuke Niwa from comment #18)
> Comment on attachment 363023 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=363023&action=review
> 
> > Source/WebCore/editing/ReplaceSelectionCommand.cpp:1340
> > +    Element* textControl = enclosingTextFormControl(positionAtStartOfInsertedContent().deepEquivalent());
> 
> Do we really need this check? Also use auto?

The check for smart paste for spaces checks to make sure this isn't a password field. I figured we just don't want any input fields at all, but I can just check for password fields specifically, it just seems like we just shouldn't even even try to put newlines of any form in an input field.

> 
> > Source/WebCore/editing/ReplaceSelectionCommand.cpp:1359
> > +    Position endUpstream = endOfInsertedContent.deepEquivalent().upstream();
> > +    
> > +    Position startDownstream = startOfInsertedContent.deepEquivalent().downstream();
> 
> These two variables are never used. Remove?
> 
> > Source/WebCore/editing/ReplaceSelectionCommand.cpp:1372
> > +            setEndingSelection(VisibleSelection(startOfInsertedContent));
> 
> This is correct use of setEndingSelection but the conversion to
> VisibleSelection is implicit
> so you can just do setEndingSelection(startOfInsertedContent).
> 
> > Source/WebCore/editing/ReplaceSelectionCommand.cpp:1382
> > +            setEndingSelection(VisibleSelection(endOfInsertedContent));
> 
> Ditto.
> 
> > Source/WebCore/editing/ReplaceSelectionCommand.cpp:1388
> > +    }
> 
> As I commented in the previous patch, we probably need to restore ending
> selection here in the case we've modified above.

Ok, that's what I thought I was doing with " m_endOfInsertedContent = newEnd.deepEquivalent();" Is that now how to set the end of the selection? This is what I thought was doing the ending selection update in the insert smart spaces code... I'm not updating the start selection, though, that is probably something that I should do as well. Or is this not the right way to do this anymore?

[EWS Watchlist]

Comment on attachment 363050 [details]
Patch

Attachment 363050 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11298821

New failing tests:
editing/pasteboard/5065605.html
editing/pasteboard/4076267-3.html
editing/pasteboard/do-not-copy-unnecessary-styles.html
editing/pasteboard/paste-text-013.html
editing/pasteboard/paste-list-003.html
editing/pasteboard/paste-text-012.html
editing/pasteboard/paste-list-002.html
editing/pasteboard/pasting-into-p-should-not-nest-p.html
editing/pasteboard/paste-text-005.html
editing/pasteboard/preserve-line-break-at-end-of-pasted-content.html
editing/pasteboard/paste-text-016.html
editing/pasteboard/avoid-copying-body-with-background.html
editing/pasteboard/paste-list-001.html
editing/pasteboard/paste-text-011.html
editing/pasteboard/paste-text-006.html
editing/pasteboard/paste-text-014.html
editing/pasteboard/paste-text-015.html
editing/pasteboard/copy-paste-pre-line-content.html
editing/pasteboard/paste-text-007.html

[EWS Watchlist]

Created attachment 363070 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Megan Gardner]

Created attachment 363148 [details]
Patch

[EWS Watchlist]

Comment on attachment 363148 [details]
Patch

Attachment 363148 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11309836

New failing tests:
editing/pasteboard/paste-text-016.html
editing/pasteboard/paste-text-007.html
editing/pasteboard/paste-text-006.html
editing/pasteboard/paste-text-005.html
editing/pasteboard/paste-text-014.html

[EWS Watchlist]

Created attachment 363170 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Ryosuke Niwa]

Comment on attachment 363148 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363148&action=review

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1355
> +static bool isNewLine(VisiblePosition &p)

Please don't use one letter variable names such as p.
Also, this is wrong style. & should be next to VisiblePosition.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1359
> +    if (isStartOfLine(p) && (startOfLine(p.next()) != startOfLine(p)))
> +        return true;
> +    return false;

It seems like should just do this in single line? There is no need for a if statement.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1367
> +    bool isFullParagraph = isStartOfParagraph(startOfInsertedContent) && isEndOfParagraph(endOfInsertedContent);

What does this check do? I don't get what you mean by "full paragraph".
We should either rid of this confusing local variable or come up with more a descriptive variable name instead.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1378
> +        if (!isNewLine(positionBeforeStart) && !isNewLine(startOfInsertedContent) && isEndOfLine(positionBeforeStart) && !isEndOfEditableOrNonEditableContent(positionAfterEnd) && !isEndOfEditableOrNonEditableContent(endOfInsertedContent)) {

It seems that we should also check whether we're at the start of the editable region or not?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1383
> +            VisiblePosition newStart = startOfInsertedContent.previous(CannotCrossEditingBoundary, &reachedBoundaryStart);
> +            if (!reachedBoundaryStart)
> +                m_startOfInsertedContent = newStart.deepEquivalent();

A better change would be to use endingSelection().deepEquivalent().
The trouble here is that startOfInsertedContent may not be in the document after executing insertParagraphSeparator().

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1387
> +    reachedBoundaryEnd = false;

Why is it okay to not re-set reachedBoundaryStart here?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1391
> +        if (isStartOfLine(positionAfterEnd) && !isEndOfLine(positionAfterEnd) && !isNewLine(positionAfterEnd) && !isNewLine(endOfInsertedContent)) {

Ditto.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1396
> +            VisiblePosition newEnd = endOfInsertedContent.next(CannotCrossEditingBoundary, &reachedBoundaryEnd);
> +            if (!reachedBoundaryEnd)
> +                m_endOfInsertedContent = newEnd.deepEquivalent();

Ditto about restoring m_endOfInsertedContent from endingSelection instead.

[Megan Gardner]

Created attachment 364468 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 364468 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364468&action=review

> Source/WebCore/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).

Nit: This line should appear before the description but after the bug URL.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:781
> +static bool isNewLine(VisiblePosition& position)

I'd prefer calling this isBlankLine.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:783
> +    return isStartOfLine(position) && (startOfLine(position.next()) != startOfLine(position));

Nit: We don't need parenthesis here.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1085
> +    if (!reachedBoundaryStart && !reachedBoundaryEnd && isNewLine(visibleInsertionPos) && isEndOfLine(positionBeforeInsertion) && isStartOfLine(positionAfterInsertion))
> +        hasBlankLinesBetweenParagraphs = true;

Let's extract this as a helper function like isBlankLineBetweenParagraphs.
Also, why don't we define a local bool like hasLineBeforePosition = isEndOfLine(positionBeforeInsertion)
so as to clarify the intent of the last two checks.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1144
> +        isList = true;
> +    }
>      else {

Nit: } else {

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1390
> +            VisiblePosition newStart = startOfInsertedContent.previous(CannotCrossEditingBoundary, &reachedBoundaryStart);

We still need to use VisiblePosition { endingSelection() }.previous() here. startOfInsertedContent could no longer be in the document.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1405
> +            VisiblePosition newEnd = endOfInsertedContent.next(CannotCrossEditingBoundary, &reachedBoundaryEnd);

Ditto; endingSelection().

> LayoutTests/ChangeLog:7
> +

Please mention which test is testing pasting into a list.
In particular, we need to make sure we cover pasting into OL, UL, and between LI's
as well as nested list items.

We also need a test case where we paste a list.

Also we need tests for pasting into table cells. Let's make sure we have some tests.

[EWS Watchlist]

Comment on attachment 364468 [details]
Patch

Attachment 364468 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11483560

New failing tests:
editing/pasteboard/paste-text-016.html

[EWS Watchlist]

Created attachment 364495 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Megan Gardner]

Created attachment 364496 [details]
Patch

[EWS Watchlist]

Comment on attachment 364496 [details]
Patch

Attachment 364496 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11484697

New failing tests:
editing/pasteboard/smart-paste-paragraph-003.html

[EWS Watchlist]

Created attachment 364509 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Megan Gardner]

Created attachment 364539 [details]
Patch

[EWS Watchlist]

Comment on attachment 364539 [details]
Patch

Attachment 364539 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11490622

New failing tests:
editing/pasteboard/smart-paste-paragraph-003.html

[EWS Watchlist]

Created attachment 364542 [details]
Archive of layout-test-results from ews103 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 364539 [details]
Patch

Attachment 364539 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11490777

New failing tests:
editing/pasteboard/smart-paste-paragraph-003.html

[EWS Watchlist]

Created attachment 364547 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Megan Gardner]

Created attachment 364548 [details]
Patch

[Megan Gardner]

<rdar://problem/48813873>

[Ryosuke Niwa]

Comment on attachment 364548 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364548&action=review

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:933
> +static bool hasBlankLineBetweenParagraphs(Position &position)

Nit: & should be next to Position as in Position&.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:937
> +    VisiblePosition visiblePosition(position);

Nit: Use { ~ }?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:939
> +    VisiblePosition positionBeforeInsertion = visiblePosition.previous(CannotCrossEditingBoundary, &reachedBoundaryStart);
> +    VisiblePosition positionAfterInsertion = visiblePosition.next(CannotCrossEditingBoundary, &reachedBoundaryStart);

It's strange to call this positionBeforeInsertion & positionAfterInsertion since we're not talking about any insertion point here.
How about previousPosition & nextPosition?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:941
> +    

Nit: whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1137
> +    bool isList = false;

isList is a bit ambiguous here. What is a list?
I think we need to give it a more describe name like isPastingIntoList or isInsertingIntoList.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1357
> +    

Nit: whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1360
> +    

Nit: whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1362
> +    if (is<HTMLInputElement>(textControl))

Do we really need this check? hasBlankLinesBetweenParagraphs should be false since there won't be an extra line no?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1364
> +    

Nit: whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1378
> +    bool isParagraph = isStartOfParagraph(startOfInsertedContent) && isEndOfParagraph(endOfInsertedContent);

What is a paragraph? How about something like isPastedContentEntireParagraphs?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1380
> +    // if we aren't pasting a paragraph, no need to attempt to insert newlines.

Nit: Capitalize "if".

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1390
> +        if (!isBlankLine(positionBeforeStart) && !isBlankLine(startOfInsertedContent) && isEndOfLine(positionBeforeStart) && !isEndOfEditableOrNonEditableContent(positionAfterEnd) && !isEndOfEditableOrNonEditableContent(endOfInsertedContent)) {

It seems that !isEndOfEditableOrNonEditableContent(positionAfterEnd) should be checked for the end case instead??

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1393
> +            VisiblePosition newStart = VisiblePosition(endingSelection().start()).previous(CannotCrossEditingBoundary, &reachedBoundaryStart);

Use endingSelection().visibleStart() instead. Also, use auto?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1398
> +    

Nit: Whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1403
> +    

Nit: Whitespace.

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1405
> +        if (isStartOfLine(positionAfterEnd) && !isEndOfLine(positionAfterEnd) && !isBlankLine(positionAfterEnd) && !isBlankLine(endOfInsertedContent)) {

Can we order the checks in the same order as the start case?
Also, don't we need to check !isEndOfEditableOrNonEditableContent(positionAfterEnd) here?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1408
> +            if (!reachedBoundaryEnd)

reachedBoundaryStart check here seems bogus. Remove?

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1409
> +                m_endOfInsertedContent = endingSelection().start();

Use visibleEnd() maybe? visibleStart() is okay too.

> LayoutTests/editing/pasteboard/smart-paste-paragraph-001.html:55
> +async function editingTest() {

Why not enable iOS editing behavior manually so that we can always test this:
internals.settings.setEditingBehavior("ios");

Ditto for all other tests.

[Megan Gardner]

Created attachment 364699 [details]
Patch

[Megan Gardner]

Created attachment 364876 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 364876 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364876&action=review

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1144
>      if ((isListHTMLElement(refNode.get()) || (isLegacyAppleStyleSpan(refNode.get()) && isListHTMLElement(refNode->firstChild())))
> -        && blockStart && blockStart->renderer()->isListItem())
> +        && blockStart && blockStart->renderer()->isListItem()) {

Why don't we do:
bool isInsertingIntoList = (isListHTMLElement(refNode.get()) || ...;
if (isInsertingIntoList)
    refNode = ...;
instead?

[Megan Gardner]

Created attachment 365049 [details]
Patch

[EWS Watchlist]

Comment on attachment 365049 [details]
Patch

Attachment 365049 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11554488

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html

[EWS Watchlist]

Created attachment 365076 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Megan Gardner]

Created attachment 365097 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 365097 [details]
Patch for landing

Clearing flags on attachment: 365097

Committed r243124: <https://trac.webkit.org/changeset/243124>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.