Closed Bug 447776 Opened 12 years ago Closed 12 years ago

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

Categories

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

x86
Windows XP
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: martijn.martijn, Assigned: smontagu)

References

Details

(Keywords: hang, testcase)

Attachments

(2 files, 4 obsolete files)

Attached file testcase
See testcase, which hangs in current trunk build.
Attached patch Fix (obsolete) — Splinter Review
Ha, good catch. I even thought about this as a case that I needed to test and then forgot about it.
Assignee: nobody → smontagu
Status: NEW → ASSIGNED
Attachment #331177 - Flags: superreview?(roc)
Attachment #331177 - Flags: review?(roc)
Duplicate of this bug: 447783
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.
Attached patch Fix v.2 (obsolete) — Splinter Review
Like so?
Attachment #331177 - Attachment is obsolete: true
Attachment #331207 - Flags: superreview?(roc)
Attachment #331207 - Flags: review?(roc)
Attachment #331177 - Flags: superreview?(roc)
Attachment #331177 - Flags: review?(roc)
Actually I think you just need to check aSuppressInitialBreak which is already passed in.
Blocks: 447884
Attached patch Fix v.3 (obsolete) — Splinter Review
Attachment #331207 - Attachment is obsolete: true
Attachment #331257 - Flags: superreview?(roc)
Attachment #331257 - Flags: review?(roc)
Attachment #331207 - Flags: superreview?(roc)
Attachment #331207 - Flags: review?(roc)
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; }
Attachment #331257 - Flags: superreview?(roc)
Attachment #331257 - Flags: superreview+
Attachment #331257 - Flags: review?(roc)
Attachment #331257 - Flags: review+
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.
You don't want if (i >= bufferStart + bufferLength) in that condition. Otherwise I guess you're right.
Attached patch v.4 (obsolete) — Splinter Review
I couldn't figure out how to do a diff ignoring whitespace with hg :(
Attachment #331257 - Attachment is obsolete: true
Attachment #331263 - Flags: superreview?(roc)
Attachment #331263 - Flags: review?(roc)
Attachment #331263 - Flags: superreview?(roc)
Attachment #331263 - Flags: superreview+
Attachment #331263 - Flags: review?(roc)
Attachment #331263 - Flags: review+
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.
Keywords: checkin-needed
Attachment #331263 - Attachment is obsolete: true
Attachment #331470 - Flags: superreview+
Attachment #331470 - Flags: review+
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
Attachment #331470 - Attachment description: Adds the testcase from here and bug 447783 as crashtests → Adds the testcase from here and bug 447783 as crashtests (checked in)
Keywords: checkin-needed
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.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Thank you for checking in.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.