Changeset 259647 in webkit


Ignore:
Timestamp:
Apr 7, 2020 11:10:10 AM (4 years ago)
Author:
rniwa@webkit.org
Message:

TextManipulationController fails to replace a paragraph that ends with a br
https://bugs.webkit.org/show_bug.cgi?id=210099

Reviewed by Wenson Hsieh.

Source/WebCore:

The bug was caused by TextManipulationController::replace not ignoring the br at the end of a paragraph
even through it doesn't appear as a token. We also need to insert this br back at the end of the paragraph
when completing the manipulation.

  • editing/TextManipulationController.cpp:

(WebCore::TextManipulationController::replace):

Tools:

Added regression tests.

  • TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:

(TextManipulation.CompleteTextManipulationReplaceMultipleSimpleParagraphsSeparatedByBR):
(TextManipulation.CompleteTextManipulationReplaceParagraphsSeparatedByWrappedBR):
(TextManipulation.CompleteTextManipulationFailWhenBRIsInserted):

Location:
trunk
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r259644 r259647  
     12020-04-07  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        TextManipulationController fails to replace a paragraph that ends with a br
     4        https://bugs.webkit.org/show_bug.cgi?id=210099
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        The bug was caused by TextManipulationController::replace not ignoring the br at the end of a paragraph
     9        even through it doesn't appear as a token. We also need to insert this br back at the end of the paragraph
     10        when completing the manipulation.
     11
     12        * editing/TextManipulationController.cpp:
     13        (WebCore::TextManipulationController::replace):
     14
    1152020-04-07  Jer Noble  <jer.noble@apple.com>
    216
  • trunk/Source/WebCore/editing/TextManipulationController.cpp

    r259620 r259647  
    488488    HashSet<Ref<Node>> excludedNodes;
    489489    HashSet<Ref<Node>> nodesToRemove;
     490    RefPtr<Node> nodeToInsertBackAtEnd;
    490491    for (; !iterator.atEnd(); iterator.advance()) {
    491492        auto content = iterator.currentContent();
     
    497498            continue;
    498499
    499         if (currentTokenIndex >= item.tokens.size())
     500        if (currentTokenIndex >= item.tokens.size()) {
     501            if (content.node && !nodeToInsertBackAtEnd && content.isTextContent && content.text == "\n") { // br
     502                nodeToInsertBackAtEnd = content.node;
     503                continue;
     504            }
    500505            return ManipulationFailureType::ContentChanged;
     506        }
    501507
    502508        auto& currentToken = item.tokens[currentTokenIndex];
     
    579585        }
    580586    }
     587    if (nodeToInsertBackAtEnd)
     588        insertions.append(NodeInsertion { nullptr, nodeToInsertBackAtEnd.releaseNonNull() });
    581589
    582590    Position insertionPoint = positionBeforeNode(firstContentNode.get()).parentAnchoredEquivalent();
  • trunk/Tools/ChangeLog

    r259641 r259647  
     12020-04-07  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        TextManipulationController fails to replace a paragraph that ends with a br
     4        https://bugs.webkit.org/show_bug.cgi?id=210099
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        Added regression tests.
     9
     10        * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
     11        (TextManipulation.CompleteTextManipulationReplaceMultipleSimpleParagraphsSeparatedByBR):
     12        (TextManipulation.CompleteTextManipulationReplaceParagraphsSeparatedByWrappedBR):
     13        (TextManipulation.CompleteTextManipulationFailWhenBRIsInserted):
     14
    1152020-04-07  Adrian Perez de Castro  <aperez@igalia.com>
    216
  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm

    r258399 r259647  
    734734}
    735735
     736TEST(TextManipulation, CompleteTextManipulationReplaceMultipleSimpleParagraphsSeparatedByBR)
     737{
     738    auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
     739    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
     740    [webView _setTextManipulationDelegate:delegate.get()];
     741
     742    [webView synchronouslyLoadHTMLString:@"<!DOCTYPE html>"
     743        "<html><body><p>helllo, wooorld<br>webKit</p></body></html>"];
     744
     745    done = false;
     746    [webView _startTextManipulationsWithConfiguration:nil completion:^{
     747        done = true;
     748    }];
     749    TestWebKitAPI::Util::run(&done);
     750
     751    auto *items = [delegate items];
     752    EXPECT_EQ(items.count, 2UL);
     753    EXPECT_EQ(items[0].tokens.count, 1UL);
     754    EXPECT_STREQ("helllo, wooorld", items[0].tokens[0].content.UTF8String);
     755    EXPECT_EQ(items[1].tokens.count, 1UL);
     756    EXPECT_STREQ("webKit", items[1].tokens[0].content.UTF8String);
     757
     758    done = false;
     759    [webView _completeTextManipulationForItems:@[(_WKTextManipulationItem *)createItem(items[0].identifier, {
     760        { items[0].tokens[0].identifier, @"Hello, World" },
     761    }), (_WKTextManipulationItem *)createItem(items[1].identifier, {
     762        { items[1].tokens[0].identifier, @"WebKit" },
     763    })] completion:^(NSArray<NSError *> *errors) {
     764        EXPECT_EQ(errors, nil);
     765        done = true;
     766    }];
     767    TestWebKitAPI::Util::run(&done);
     768    EXPECT_WK_STREQ("<p>Hello, World<br>WebKit</p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
     769}
     770
     771TEST(TextManipulation, CompleteTextManipulationReplaceParagraphsSeparatedByWrappedBR)
     772{
     773    auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
     774    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
     775    [webView _setTextManipulationDelegate:delegate.get()];
     776
     777    [webView synchronouslyLoadHTMLString:@"<!DOCTYPE html>"
     778        "<html><body><p>earth, <b>hey<br></b>webKit</p></body></html>"];
     779
     780    done = false;
     781    [webView _startTextManipulationsWithConfiguration:nil completion:^{
     782        done = true;
     783    }];
     784    TestWebKitAPI::Util::run(&done);
     785
     786    auto *items = [delegate items];
     787    EXPECT_EQ(items.count, 2UL);
     788    EXPECT_EQ(items[0].tokens.count, 2UL);
     789    EXPECT_STREQ("earth, ", items[0].tokens[0].content.UTF8String);
     790    EXPECT_STREQ("hey", items[0].tokens[1].content.UTF8String);
     791    EXPECT_EQ(items[1].tokens.count, 1UL);
     792    EXPECT_STREQ("webKit", items[1].tokens[0].content.UTF8String);
     793
     794    done = false;
     795    [webView _completeTextManipulationForItems:@[(_WKTextManipulationItem *)createItem(items[0].identifier, {
     796        { items[0].tokens[1].identifier, @"Hello, " },
     797        { items[0].tokens[0].identifier, @"World" },
     798    }), (_WKTextManipulationItem *)createItem(items[1].identifier, {
     799        { items[1].tokens[0].identifier, @"WebKit" },
     800    })] completion:^(NSArray<NSError *> *errors) {
     801        EXPECT_EQ(errors, nil);
     802        done = true;
     803    }];
     804    TestWebKitAPI::Util::run(&done);
     805    EXPECT_WK_STREQ("<p><b>Hello, </b>World<br>WebKit</p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
     806}
     807
     808TEST(TextManipulation, CompleteTextManipulationFailWhenBRIsInserted)
     809{
     810    auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
     811    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
     812    [webView _setTextManipulationDelegate:delegate.get()];
     813
     814    [webView synchronouslyLoadHTMLString:@"<!DOCTYPE html><html><body><p>helllo, <b>worrld</b></p></body></html>"];
     815
     816    done = false;
     817    [webView _startTextManipulationsWithConfiguration:nil completion:^{
     818        done = true;
     819    }];
     820    TestWebKitAPI::Util::run(&done);
     821
     822    auto *items = [delegate items];
     823    EXPECT_EQ(items.count, 1UL);
     824    EXPECT_EQ(items[0].tokens.count, 2UL);
     825    EXPECT_STREQ("helllo, ", items[0].tokens[0].content.UTF8String);
     826    EXPECT_STREQ("worrld", items[0].tokens[1].content.UTF8String);
     827
     828    [webView stringByEvaluatingJavaScript:@"document.querySelector('b').before(document.createElement('br'))"];
     829
     830    done = false;
     831    __block auto item = createItem(items[0].identifier, {
     832        { items[0].tokens[0].identifier, @"Hello, " },
     833        { items[0].tokens[1].identifier, @"World" },
     834    });
     835    [webView _completeTextManipulationForItems:@[item.get()] completion:^(NSArray<NSError *> *errors) {
     836        EXPECT_EQ(errors.count, 1UL);
     837        EXPECT_EQ(errors.firstObject.domain, _WKTextManipulationItemErrorDomain);
     838        EXPECT_EQ(errors.firstObject.code, _WKTextManipulationItemErrorContentChanged);
     839        EXPECT_EQ(errors.firstObject.userInfo[_WKTextManipulationItemErrorItemKey], item.get());
     840        done = true;
     841    }];
     842    TestWebKitAPI::Util::run(&done);
     843    EXPECT_WK_STREQ("<p>helllo, <br><b>worrld</b></p>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
     844}
     845
    736846TEST(TextManipulation, CompleteTextManipulationShouldPreserveImagesAsExcludedTokens)
    737847{
Note: See TracChangeset for help on using the changeset viewer.