Bug 169516 - Simple line layout: Paginated content is not painted properly when font overflows line height.
Summary: Simple line layout: Paginated content is not painted properly when font overf...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-11 20:05 PST by zalan
Modified: 2017-03-12 20:16 PDT (History)
5 users (show)

See Also:


Attachments
Patch (8.30 KB, patch)
2017-03-11 20:40 PST, zalan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (1.26 MB, application/zip)
2017-03-11 22:00 PST, Build Bot
no flags Details
Patch (9.11 KB, patch)
2017-03-12 10:50 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (8.92 KB, patch)
2017-03-12 19:35 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2017-03-11 20:05:53 PST
font size -> big number.
line height -> small number 
-> lines overflow each other.
Comment 1 zalan 2017-03-11 20:40:08 PST
Created attachment 304183 [details]
Patch
Comment 2 Build Bot 2017-03-11 22:00:22 PST
Comment on attachment 304183 [details]
Patch

Attachment 304183 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3296283

New failing tests:
fast/multicol/simple-line-layout-line-index-after-strut.html
Comment 3 Build Bot 2017-03-11 22:00:25 PST
Created attachment 304185 [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.11.6
Comment 4 zalan 2017-03-12 10:50:57 PDT
Created attachment 304197 [details]
Patch
Comment 5 Antti Koivisto 2017-03-12 13:40:44 PDT
Comment on attachment 304197 [details]
Patch

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

> Source/WebCore/rendering/SimpleLineLayoutResolver.cpp:145
> +    for (int lineIndex = strut.lineBreak; lineIndex < (int)m_layout.lineCount(); ++lineIndex) {

Why int? Can strut.lineBreak and so lineIndex be negative? If it can should that case be handled separately as the rest of the function doesn't seem safe with negative values.

static_cast<int> would be more WebKit style than c-style cast (if we really need it).

> Source/WebCore/rendering/SimpleLineLayoutResolver.cpp:147
> +        if (strutIndex < struts.size() && struts.at(strutIndex).lineBreak == (unsigned)lineIndex)

Same here.

> Source/WebCore/rendering/SimpleLineLayoutResolver.cpp:152
> +            lastIndexCandidate = lineIndex;

If lineIndex was negative wouldn't coercing it to unsigned cause problems?
Comment 6 zalan 2017-03-12 19:31:34 PDT
(In reply to comment #5)
> Comment on attachment 304197 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=304197&action=review
> 
> > Source/WebCore/rendering/SimpleLineLayoutResolver.cpp:145
> > +    for (int lineIndex = strut.lineBreak; lineIndex < (int)m_layout.lineCount(); ++lineIndex) {
> 
> Why int? Can strut.lineBreak and so lineIndex be negative? If it can should
> that case be handled separately as the rest of the function doesn't seem
> safe with negative values.
> 
> static_cast<int> would be more WebKit style than c-style cast (if we really
> need it).
> 
> > Source/WebCore/rendering/SimpleLineLayoutResolver.cpp:147
> > +        if (strutIndex < struts.size() && struts.at(strutIndex).lineBreak == (unsigned)lineIndex)
> 
> Same here.
> 
> > Source/WebCore/rendering/SimpleLineLayoutResolver.cpp:152
> > +            lastIndexCandidate = lineIndex;
> 
> If lineIndex was negative wouldn't coercing it to unsigned cause problems?
Yea, you are right. I am not sure why I have it like that. Must be some leftover.
Comment 7 zalan 2017-03-12 19:35:11 PDT
Created attachment 304215 [details]
Patch
Comment 8 WebKit Commit Bot 2017-03-12 20:16:47 PDT
Comment on attachment 304215 [details]
Patch

Clearing flags on attachment: 304215

Committed r213779: <http://trac.webkit.org/changeset/213779>
Comment 9 WebKit Commit Bot 2017-03-12 20:16:51 PDT
All reviewed patches have been landed.  Closing bug.