Changeset 254153 in webkit


Ignore:
Timestamp:
Jan 7, 2020 1:26:54 PM (4 years ago)
Author:
dbates@webkit.org
Message:

First character in each word-wrapped line has incorrect character rect when requested range spans multiple lines
​https://bugs.webkit.org/show_bug.cgi?id=205842
<rdar://problem/56884325>

Reviewed by Zalan Bujtas.

Source/WebCore:

Adds a new BoundingRectBehavior enumarator, IgnoreEmptyTextSelections, to ignore line boxes
that are not selected by the specified range when computing the bounding box for it via Range::absoluteBoundingBox().

A line box is said to be selected if there is at least one character in the specified character
range. So, a range whose start position coincides with the edge of a line box does not select the
box. However such ranges are considered to select such boxes when passed to web-exposed APIs
{Element, Range}.getClientRects() and {Element, Range}.getBoundingClientRect(). These ranges
produce empty client rectangles and these empty rectangles effect the computation of the bounding
client rect. This is all speced behavior.

When computing the glpyh bounding box for a document context request, these empty rectangles are
not meaningful and cause weird results: the empty rect is unioned with the rect for the next selected
character producing a rectangle that overlaps two lines. Ignoring them makes things behave more like
NSLayoutManager.

  • dom/Range.cpp:

(WebCore::Range::absoluteBoundingBox const):
(WebCore::Range::absoluteRectsForRangeInText const):
(WebCore::Range::absoluteTextQuads const):

  • dom/Range.h:
  • rendering/RenderText.cpp:

(WebCore::RenderText::absoluteQuadsForRange const):

  • rendering/RenderText.h:
  • rendering/RenderTextLineBoxes.cpp:

(WebCore::RenderTextLineBoxes::absoluteQuadsForRange const):
Pass an option to ignore empty text selections through. By default Range::absoluteBoundingBox() takes
an empty set of BoundingRectBehavior enumerators to keep its current behavior.

(WebCore::RenderTextLineBoxes::absoluteRectsForRange const): Added a boolean as to whether to
ignore empty selections. If enabled, skip all boxes that are not selected by the specified
character range.

  • rendering/RenderTextLineBoxes.h:
  • rendering/SimpleLineLayoutFunctions.cpp:

(WebCore::SimpleLineLayout::collectAbsoluteQuadsForRange): Ditto.

  • rendering/SimpleLineLayoutFunctions.h:

Source/WebKit:

Pass BoundingRectBehavior::IgnoreEmptyTextSelections to Range::absoluteBoundingBox() to compute
the glyph bounding box ignoring empty text selections that would typically occur if the start of
the range coincide with the edge of a line box. This makes the behavior more consistent with
the behavior of NSLayoutManager.

  • WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::requestDocumentEditingContext):

Tools:

Add a new test.

  • TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:

(TEST):

Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r254151 r254153  
     12020-01-07  Daniel Bates  <dabates@apple.com>
     2
     3        First character in each word-wrapped line has incorrect character rect when requested range spans multiple lines
     4        https://bugs.webkit.org/show_bug.cgi?id=205842
     5        <rdar://problem/56884325>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Adds a new BoundingRectBehavior enumarator, IgnoreEmptyTextSelections, to ignore line boxes
     10        that are not selected by the specified range when computing the bounding box for it via Range::absoluteBoundingBox().
     11
     12        A line box is said to be selected if there is at least one character in the specified character
     13        range. So, a range whose start position coincides with the edge of a line box does not select the
     14        box. However such ranges are considered to select such boxes when passed to web-exposed APIs
     15        {Element, Range}.getClientRects() and {Element, Range}.getBoundingClientRect(). These ranges
     16        produce empty client rectangles and these empty rectangles effect the computation of the bounding
     17        client rect. This is all speced behavior.
     18
     19        When computing the glpyh bounding box for a document context request, these empty rectangles are
     20        not meaningful and cause weird results: the empty rect is unioned with the rect for the next selected
     21        character producing a rectangle that overlaps two lines. Ignoring them makes things behave more like
     22        NSLayoutManager.
     23
     24        * dom/Range.cpp:
     25        (WebCore::Range::absoluteBoundingBox const):
     26        (WebCore::Range::absoluteRectsForRangeInText const):
     27        (WebCore::Range::absoluteTextQuads const):
     28        * dom/Range.h:
     29        * rendering/RenderText.cpp:
     30        (WebCore::RenderText::absoluteQuadsForRange const):
     31        * rendering/RenderText.h:
     32        * rendering/RenderTextLineBoxes.cpp:
     33        (WebCore::RenderTextLineBoxes::absoluteQuadsForRange const):
     34        Pass an option to ignore empty text selections through. By default Range::absoluteBoundingBox() takes
     35        an empty set of BoundingRectBehavior enumerators to keep its current behavior.
     36
     37        (WebCore::RenderTextLineBoxes::absoluteRectsForRange const): Added a boolean as to whether to
     38        ignore empty selections. If enabled, skip all boxes that are not selected by the specified
     39        character range.
     40        * rendering/RenderTextLineBoxes.h:
     41        * rendering/SimpleLineLayoutFunctions.cpp:
     42        (WebCore::SimpleLineLayout::collectAbsoluteQuadsForRange): Ditto.
     43        * rendering/SimpleLineLayoutFunctions.h:
     44
    1452020-01-07  Ryan Haddad  <ryanhaddad@apple.com>
    246

  • trunk/Source/WebCore/dom/Range.cpp

    r254151 r254153  
    11441144}
    11451145
    1146 IntRect Range::absoluteBoundingBox() const
     1146IntRect Range::absoluteBoundingBox(OptionSet<BoundingRectBehavior> rectOptions) const
    11471147{
    11481148    IntRect result;
    11491149    Vector<IntRect> rects;
    1150     absoluteTextRects(rects);
     1150    bool useSelectionHeight = false;
     1151    RangeInFixedPosition* inFixed = nullptr;
     1152    absoluteTextRects(rects, useSelectionHeight, inFixed, rectOptions);
    11511153    for (auto& rect : rects)
    11521154        result.unite(rect);
    … …  
    11591161    unsigned endOffset = node == &endContainer() ? m_end.offset() : std::numeric_limits<unsigned>::max();
    11601162
    1161     auto textQuads = renderText.absoluteQuadsForRange(startOffset, endOffset, useSelectionHeight, &isFixed);
     1163    auto textQuads = renderText.absoluteQuadsForRange(startOffset, endOffset, useSelectionHeight, rectOptions.contains(BoundingRectBehavior::IgnoreEmptyTextSelections), &isFixed);
    11621164
    11631165    if (rectOptions.contains(BoundingRectBehavior::RespectClipping)) {
    … …  
    12281230            unsigned startOffset = node == &startContainer() ? m_start.offset() : 0;
    12291231            unsigned endOffset = node == &endContainer() ? m_end.offset() : std::numeric_limits<unsigned>::max();
    1230             quads.appendVector(downcast<RenderText>(*renderer).absoluteQuadsForRange(startOffset, endOffset, useSelectionHeight, &isFixed));
     1232            quads.appendVector(downcast<RenderText>(*renderer).absoluteQuadsForRange(startOffset, endOffset, useSelectionHeight, false /* ignoreEmptyTextSelections */, &isFixed));
    12311233        } else
    12321234            continue;

  • trunk/Source/WebCore/dom/Range.h

    r254151 r254153  
    121121        UseVisibleBounds = 1 << 1,
    122122        IgnoreTinyRects = 1 << 2,
     123        IgnoreEmptyTextSelections = 1 << 3, // Do not return empty text rectangles, which is required for Element.getClientRect() conformance.
    123124    };
    124125
    125126    // Not transform-friendly
    126127    WEBCORE_EXPORT void absoluteTextRects(Vector<IntRect>&, bool useSelectionHeight = false, RangeInFixedPosition* = nullptr, OptionSet<BoundingRectBehavior> = { }) const;
    127     WEBCORE_EXPORT IntRect absoluteBoundingBox() const;
     128    WEBCORE_EXPORT IntRect absoluteBoundingBox(OptionSet<BoundingRectBehavior> = { }) const;
    128129
    129130    // Transform-friendly

  • trunk/Source/WebCore/rendering/RenderText.cpp

    r254151 r254153  
    432432}
    433433
    434 Vector<FloatQuad> RenderText::absoluteQuadsForRange(unsigned start, unsigned end, bool useSelectionHeight, bool* wasFixed) const
     434Vector<FloatQuad> RenderText::absoluteQuadsForRange(unsigned start, unsigned end, bool useSelectionHeight, bool ignoreEmptyTextSelections, bool* wasFixed) const
    435435{
    436436    // Work around signed/unsigned issues. This function takes unsigneds, and is often passed UINT_MAX
    … …  
    444444    end = std::min(end, static_cast<unsigned>(INT_MAX));
    445445    if (simpleLineLayout() && !useSelectionHeight)
    446         return collectAbsoluteQuadsForRange(*this, start, end, *simpleLineLayout(), wasFixed);
     446        return collectAbsoluteQuadsForRange(*this, start, end, *simpleLineLayout(), ignoreEmptyTextSelections, wasFixed);
    447447    const_cast<RenderText&>(*this).ensureLineBoxes();
    448     return m_lineBoxes.absoluteQuadsForRange(*this, start, end, useSelectionHeight, wasFixed);
     448    return m_lineBoxes.absoluteQuadsForRange(*this, start, end, useSelectionHeight, ignoreEmptyTextSelections, wasFixed);
    449449}
    450450

  • trunk/Source/WebCore/rendering/RenderText.h

    r254151 r254153  
    7676
    7777    void absoluteQuads(Vector<FloatQuad>&, bool* wasFixed) const final;
    78     Vector<FloatQuad> absoluteQuadsForRange(unsigned startOffset = 0, unsigned endOffset = UINT_MAX, bool useSelectionHeight = false, bool* wasFixed = nullptr) const;
     78    Vector<FloatQuad> absoluteQuadsForRange(unsigned startOffset = 0, unsigned endOffset = UINT_MAX, bool useSelectionHeight = false, bool ignoreEmptyTextSelections = false, bool* wasFixed = nullptr) const;
    7979
    8080    Vector<FloatQuad> absoluteQuadsClippedToEllipsis() const;

  • trunk/Source/WebCore/rendering/RenderTextLineBoxes.cpp

    r254151 r254153  
    430430}
    431431
    432 Vector<FloatQuad> RenderTextLineBoxes::absoluteQuadsForRange(const RenderText& renderer, unsigned start, unsigned end, bool useSelectionHeight, bool* wasFixed) const
     432Vector<FloatQuad> RenderTextLineBoxes::absoluteQuadsForRange(const RenderText& renderer, unsigned start, unsigned end, bool useSelectionHeight, bool ignoreEmptyTextSelections, bool* wasFixed) const
    433433{
    434434    Vector<FloatQuad> quads;
    435435    for (auto* box = m_first; box; box = box->nextTextBox()) {
     436        if (ignoreEmptyTextSelections && !box->isSelected(start, end))
     437            continue;
    436438        if (start <= box->start() && box->end() <= end) {
    437439            FloatRect boundaries = box->calculateBoundaries();
    … …  
    458460Vector<IntRect> RenderTextLineBoxes::absoluteRectsForRange(const RenderText& renderer, unsigned start, unsigned end, bool useSelectionHeight, bool* wasFixed) const
    459461{
    460     return absoluteQuadsForRange(renderer, start, end, useSelectionHeight, wasFixed).map([](auto& quad) { return quad.enclosingBoundingBox(); });
     462    return absoluteQuadsForRange(renderer, start, end, useSelectionHeight, false /* ignoreEmptyTextSelections */, wasFixed).map([](auto& quad) { return quad.enclosingBoundingBox(); });
    461463}
    462464

  • trunk/Source/WebCore/rendering/RenderTextLineBoxes.h

    r254151 r254153  
    6767    enum ClippingOption { NoClipping, ClipToEllipsis };
    6868    Vector<FloatQuad> absoluteQuads(const RenderText&, bool* wasFixed, ClippingOption) const;
    69     Vector<FloatQuad> absoluteQuadsForRange(const RenderText&, unsigned start, unsigned end, bool useSelectionHeight, bool* wasFixed) const;
     69    Vector<FloatQuad> absoluteQuadsForRange(const RenderText&, unsigned start, unsigned end, bool useSelectionHeight, bool ignoreEmptyTextSelections, bool* wasFixed) const;
    7070    Vector<IntRect> absoluteRectsForRange(const RenderText&, unsigned start, unsigned end, bool useSelectionHeight, bool* wasFixed) const;
    7171

  • trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp

    r254151 r254153  
    210210}
    211211
    212 Vector<FloatQuad> collectAbsoluteQuadsForRange(const RenderObject& renderer, unsigned start, unsigned end, const Layout& layout, bool* wasFixed)
     212Vector<FloatQuad> collectAbsoluteQuadsForRange(const RenderObject& renderer, unsigned start, unsigned end, const Layout& layout, bool ignoreEmptyTextSelections, bool* wasFixed)
    213213{
    214214    auto& style = downcast<RenderBlockFlow>(*renderer.parent()).style();
    … …  
    216216    auto& resolver = layout.runResolver();
    217217    for (auto run : resolver.rangeForRendererWithOffsets(renderer, start, end)) {
     218        if (ignoreEmptyTextSelections && run.start() == run.end())
     219            continue;
    218220        // This run is fully contained.
    219221        if (start <= run.start() && end >= run.end()) {

  • trunk/Source/WebCore/rendering/SimpleLineLayoutFunctions.h

    r254151 r254153  
    5252Vector<FloatQuad> collectAbsoluteQuads(const RenderObject&, const Layout&, bool* wasFixed);
    5353unsigned textOffsetForPoint(const LayoutPoint&, const RenderText&, const Layout&);
    54 Vector<FloatQuad> collectAbsoluteQuadsForRange(const RenderObject&, unsigned start, unsigned end, const Layout&, bool* wasFixed);
     54Vector<FloatQuad> collectAbsoluteQuadsForRange(const RenderObject&, unsigned start, unsigned end, const Layout&, bool ignoreEmptyTextSelections, bool* wasFixed);
    5555
    5656LayoutUnit lineHeightFromFlow(const RenderBlockFlow&);

  • trunk/Source/WebKit/ChangeLog

    r254151 r254153  
     12020-01-07  Daniel Bates  <dabates@apple.com>
     2
     3        First character in each word-wrapped line has incorrect character rect when requested range spans multiple lines
     4        https://bugs.webkit.org/show_bug.cgi?id=205842
     5        <rdar://problem/56884325>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Pass BoundingRectBehavior::IgnoreEmptyTextSelections to Range::absoluteBoundingBox() to compute
     10        the glyph bounding box ignoring empty text selections that would typically occur if the start of
     11        the range coincide with the edge of a line box. This makes the behavior more consistent with
     12        the behavior of NSLayoutManager.
     13
     14        * WebProcess/WebPage/ios/WebPageIOS.mm:
     15        (WebKit::WebPage::requestDocumentEditingContext):
     16
    1172020-01-07  Ryan Haddad  <ryanhaddad@apple.com>
    218

  • trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

    r254151 r254153  
    40624062            }
    40634063
    4064             rects.append({ contextIterator.range()->absoluteBoundingBox(), { currentLocation, 1 } });
     4064            rects.append({ contextIterator.range()->absoluteBoundingBox(Range::BoundingRectBehavior::IgnoreEmptyTextSelections), { currentLocation, 1 } });
    40654065            currentLocation++;
    40664066            contextIterator.advance(1);

  • trunk/Tools/ChangeLog

    r254151 r254153  
     12020-01-07  Daniel Bates  <dabates@apple.com>
     2
     3        First character in each word-wrapped line has incorrect character rect when requested range spans multiple lines
     4        https://bugs.webkit.org/show_bug.cgi?id=205842
     5        <rdar://problem/56884325>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Add a new test.
     10
     11        * TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm:
     12        (TEST):
     13
    1142020-01-07  Ryan Haddad  <ryanhaddad@apple.com>
    215

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/DocumentEditingContext.mm

    r254151 r254153  
    444444}
    445445
     446TEST(DocumentEditingContext, RequestRectsInTextAreaAcrossWordWrappedLine)
     447{
     448    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
     449    // Use "padding: 0" as the default user-agent stylesheet can effect text wrapping.
     450    [webView synchronouslyLoadHTMLString:applyAhemStyle(@"<textarea id='test' style='width: 26em; padding: 0'>The quick brown fox jumps over the lazy dog.</textarea>")]; // Word wraps "over" onto next line
     451    [webView stringByEvaluatingJavaScript:@"test.focus(); test.setSelectionRange(25, 25)"]; // Place caret after 's' in "jumps".
     452
     453    auto *context = [webView synchronouslyRequestDocumentContext:makeRequest(UIWKDocumentRequestText | UIWKDocumentRequestRects, UITextGranularityWord, 2)];
     454    EXPECT_NOT_NULL(context);
     455    EXPECT_NSSTRING_EQ("fox jumps", context.contextBefore);
     456    EXPECT_NSSTRING_EQ(" over the", context.contextAfter);
     457    auto *textRects = [context textRects];
     458    EXPECT_EQ(18U, textRects.count);
     459    if (textRects.count >= 18) {
     460        CGFloat x = 401;
     461        EXPECT_EQ(CGRectMake(x + 0 * glyphWidth, 3, 25, 25), textRects[0].CGRectValue); // f
     462        EXPECT_EQ(CGRectMake(x + 1 * glyphWidth, 3, 25, 25), textRects[1].CGRectValue); // o
     463        EXPECT_EQ(CGRectMake(x + 2 * glyphWidth, 3, 25, 25), textRects[2].CGRectValue); // x
     464        EXPECT_EQ(CGRectMake(x + 3 * glyphWidth, 3, 25, 25), textRects[3].CGRectValue); //
     465        EXPECT_EQ(CGRectMake(x + 4 * glyphWidth, 3, 25, 25), textRects[4].CGRectValue); // j
     466        EXPECT_EQ(CGRectMake(x + 5 * glyphWidth, 3, 25, 25), textRects[5].CGRectValue); // u
     467        EXPECT_EQ(CGRectMake(x + 6 * glyphWidth, 3, 25, 25), textRects[6].CGRectValue); // m
     468        EXPECT_EQ(CGRectMake(x + 7 * glyphWidth, 3, 25, 25), textRects[7].CGRectValue); // p
     469        EXPECT_EQ(CGRectMake(x + 8 * glyphWidth, 3, 25, 25), textRects[8].CGRectValue); // s
     470        EXPECT_EQ(CGRectMake(x + 9 * glyphWidth, 3, 23, 25), textRects[9].CGRectValue); //
     471
     472        x = 1;
     473        EXPECT_EQ(CGRectMake(x + 0 * glyphWidth, 28, 25, 25), textRects[10].CGRectValue); // o
     474        EXPECT_EQ(CGRectMake(x + 1 * glyphWidth, 28, 25, 25), textRects[11].CGRectValue); // v
     475        EXPECT_EQ(CGRectMake(x + 2 * glyphWidth, 28, 25, 25), textRects[12].CGRectValue); // e
     476        EXPECT_EQ(CGRectMake(x + 3 * glyphWidth, 28, 25, 25), textRects[13].CGRectValue); // r
     477        EXPECT_EQ(CGRectMake(x + 4 * glyphWidth, 28, 25, 25), textRects[14].CGRectValue); //
     478        EXPECT_EQ(CGRectMake(x + 5 * glyphWidth, 28, 25, 25), textRects[15].CGRectValue); // t
     479        EXPECT_EQ(CGRectMake(x + 6 * glyphWidth, 28, 25, 25), textRects[16].CGRectValue); // h
     480        EXPECT_EQ(CGRectMake(x + 7 * glyphWidth, 28, 25, 25), textRects[17].CGRectValue); // e
     481    }
     482}
     483
    446484// MARK: Tests using word granularity
    447485
Note: See TracChangeset for help on using the changeset viewer.