Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add failing test that highlight the infinite loop issue in the inline box layout algorithm #482 #491

Merged
merged 1 commit into from
May 26, 2020
Merged

add failing test that highlight the infinite loop issue in the inline box layout algorithm #482 #491

merged 1 commit into from
May 26, 2020

Conversation

syjer
Copy link
Contributor

@syjer syjer commented May 26, 2020 •
edited
Loading

Hi @danfickle , I've added another failing test that cause an infinite loop somewhere in the table layout algorithm. the inline box layout algorithm

I've finally managed to reduce it in a more minimal case. As described in #482 (comment) there are multiple elements in play here:

  • the fonts seems to have an impact
  • it seems to be still a SOFT_HYPHEN issue
  • the defined width of the td element seems to have an impact too

I'll try to understand what is true culprit :)

@syjer
Copy link
Contributor Author

syjer commented May 26, 2020 •
edited
Loading

Additional info, it's still stuck inside https://github.com/danfickle/openhtmltopdf/blob/open-dev-v1/openhtmltopdf-core/src/main/java/com/openhtmltopdf/layout/InlineBoxing.java#L160

It seems that the SOFT_HYPHEN character with the Liberation Sans font has a length which is more than 0.

@syjer syjer changed the title add failing test that highlight the infinite loop issue in table layout #482 add failing test that highlight the infinite loop issue in the inline box layout algorithm #482 May 26, 2020
@danfickle
Copy link
Owner

Thanks @syjer, I'll merge and have a look right away.

@danfickle danfickle merged commit ba8dc5c into danfickle:open-dev-v1 May 26, 2020
@syjer
Copy link
Contributor Author

syjer commented May 26, 2020 •
edited
Loading

hi @danfickle , I've narrowed down a little bit more :

the final file is

<html>
<head>
<style>
@page  {
    size: 30px 30px;
    margin: 0;
    padding:0;
}
    .content {
        font-family: 'Liberation Sans', sans-serif;
        word-wrap: break-word;
        width:10px;
    }
</style>
</head>
<body>
<div class="content">­</div>
</body>
</html>

danfickle added a commit that referenced this pull request May 26, 2020
@danfickle
#482 #491 #483 Fix infinite loop bug with soft-hypen plus inserted hy…
b885384 
…phen overflowing line.

The core problem seems to be the under-reporting of width when we have a soft hyphen that is found to be unbreakable.
@danfickle
Copy link
Owner

I've committed a fix but I still don't think it is robust enough. The problem is we have pseudo code like this:

while (reported width less than line width) {
   rerun word breaking algorithm
}

So that if the word breaking algorithm measures and determines it's impossible but under reports the width an infinite loop bug will occur. This was happening because the reported width was about 6px but the measured width with the inserted hyphen was twice this.

A better solution would be to go ahead and output the rest on a new line if the first run reports a width less than line width (ie. DRY).

I'll try this tomorrow. In the meantime, I'll add a comment in the release issue about break-word not being suitable for production.

P.S @syjer you were spot on about the width of a soft-hyphen being a problem. Thanks.

@danfickle danfickle mentioned this pull request May 26, 2020
Open
danfickle added a commit that referenced this pull request May 27, 2020
@danfickle
#482 #483 #491 Make word breaker testable and start writing tests.
430edfb
danfickle added a commit that referenced this pull request Jun 1, 2020
@danfickle
#482 #483 #491 Loads more tests for word breaker.
15319be 
Plus minor behaviour change for word break method to avoid setting ends-on -soft-hyphen flag for soft hyphen at end of box.
danfickle added a commit that referenced this pull request Jun 7, 2020
@danfickle
#482 #483 #491 Add test to prove that too long words fall below floats.
f251520 
This is to make sure infinite loop fixes do not break this functionality.
danfickle added a commit that referenced this pull request Jun 7, 2020
@danfickle
#429 #482 #483 #491 Safety valve on unbreakable word reattempts on ne…
990012c 
…w line.

With test to prove that we don't trigger safety valve with lots of unbreakable words. Safety valve should only be triggered if there is still a bug in our code.
danfickle added a commit that referenced this pull request Jun 9, 2020
@danfickle
#482 #483 #491 Basic fuzz testing of the line breaking algorithms.
5eb064f 
This should ensure no infinite loop bugs creep in over time.
danfickle added a commit that referenced this pull request Jun 13, 2020
@danfickle
Merge pull request #492 from danfickle/robust_breaker
9ab4c10 
#482 #483 #491 Make word breaker testable and start writing tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@syjer @danfickle