Bug 226518

Summary: Incorrect text selection when crossing flex item boundary
Product: WebKit Reporter: zalan <zalan>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
[fast-cq] Patch none

Description zalan 2021-06-01 16:26:40 PDT
<rdar://59487603>
Comment 1 zalan 2021-06-01 17:05:44 PDT
Created attachment 430307 [details]
Patch
Comment 2 zalan 2021-06-01 17:38:05 PDT
Will look into the EWS results whether they are actual failures.
Comment 3 Simon Fraser (smfr) 2021-06-01 20:40:47 PDT
Comment on attachment 430307 [details]
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:2260
> +            if (is<RenderBlockFlow>(renderer))
> +                logicalBottom += downcast<RenderBlockFlow>(renderer).lowestFloatLogicalBottom();

Since you will be blamed for this now, maybe look into the history of why it's this way, and add a FIXME if we think it's wrong.

> Source/WebCore/rendering/RenderBlock.cpp:2264
> +        if (pointInLogicalContents.y() > logicalBottomForCandidateBox || (!blocksAreFlipped && pointInLogicalContents.y() == logicalBottomForCandidateBox))

The `pointInLogicalContents.y() == logicalBottomForCandidateBox` test is very confusing here.
Comment 4 zalan 2021-06-02 18:59:10 PDT
Created attachment 430433 [details]
Patch
Comment 5 zalan 2021-06-02 19:17:34 PDT
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=430307&action=review
> 
> > Source/WebCore/rendering/RenderBlock.cpp:2260
> > +            if (is<RenderBlockFlow>(renderer))
> > +                logicalBottom += downcast<RenderBlockFlow>(renderer).lowestFloatLogicalBottom();
> 
> Since you will be blamed for this now, maybe look into the history of why
> it's this way, and add a FIXME if we think it's wrong.
> 
fixed in https://trac.webkit.org/changeset/278379/webkit
Comment 6 Simon Fraser (smfr) 2021-06-02 21:13:45 PDT
Comment on attachment 430433 [details]
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:2188
> +    if (isTable() || isFlexibleBoxIncludingDeprecated() || isRenderGrid())

Should you add grid and deprecated flex box tests?
Comment 7 zalan 2021-06-03 09:47:34 PDT
Created attachment 430478 [details]
Patch
Comment 8 zalan 2021-06-03 10:50:48 PDT
Created attachment 430481 [details]
[fast-cq] Patch
Comment 9 EWS 2021-06-03 10:53:23 PDT
Committed r278411 (238436@main): <https://commits.webkit.org/238436@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430481 [details].
Comment 10 Radar WebKit Bug Importer 2021-06-03 10:54:16 PDT
<rdar://problem/78825560>