447776 - Hang with word-wrap: break-word and width: 0px

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which hangs in current trunk build.

Comment 1

[Simon Montagu :smontagu]

Attachment: Fix

Ha, good catch. I even thought about this as a case that I needed to test and then forgot about it.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is this correct? If you have something like

<em>hello</em>kitty

there should be a break opportunity at the start of "kitty". You may need to take into account the start-of-line information that nsTextFrameThebes has.

Comment 4

[Simon Montagu :smontagu]

Attachment: Fix v.2

Like so?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Actually I think you just need to check aSuppressInitialBreak which is already passed in.

Comment 6

[aja+bugzilla]

FYI: See https://bugzilla.mozilla.org/show_bug.cgi?id=447884#c13

Comment 7

[Simon Montagu :smontagu]

Attachment: Fix v.3

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 331257 [details] [diff] [review]
Fix v.3

You should be able to rearrange the code so that the   (!aSuppressInitialBreak || i > aStart)
condition only appears once. I think I'd set lineBreakHere and wordWrapping to their values ignoring that condition, and then do
  if (aSuppressInitialBreak && i == aStart) { lineBreakHere = wordWrapping = PR_FALSE; }

Comment 9

[Simon Montagu :smontagu]

Or how about putting the whole first section of the loop (up to the end of the |if (lineBreak || hyphenation || wordWrapping { ... } | block) inside |if (!aSuppressInitialBreak || i > aStart)| ? We're not going to set a hyphenation break at the beginning of the line either.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You don't want if (i >= bufferStart + bufferLength) in that condition. Otherwise I guess you're right.

Comment 11

[Simon Montagu :smontagu]

Attachment: v.4

I couldn't figure out how to do a diff ignoring whitespace with hg :(

Comment 12

[Simon Montagu :smontagu]

This blocks critical bug 447884, and I'm unlikely to have a block of free time long enough to check in for the next day or two, so marking checkin-needed.

Comment 13

[Simon Montagu :smontagu]

Attachment: Adds the testcase from here and bug 447783 as crashtests (checked in)

Comment 14

[Ian Neal]

Comment on attachment 331470 [details] [diff] [review]
Adds the testcase from here and bug 447783 as crashtests (checked in)

http://hg.mozilla.org/mozilla-central/index.cgi/rev/7429393f48c1

Comment 15

[u88484]

Verified fixed Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/2008072710 Minefield/3.1a2pre.  Checked the dependent bug and other sites that I knew where causing crashes due to this bug.  All is good.

Comment 16

[Simon Montagu :smontagu]

Thank you for checking in.