42422 - Using :before & :first-letter together cause crash

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[97Ryan01]

Using the two CSS2 pseudo-classes :before and :first-letter crash Mozilla.

Comment 1

[97Ryan01]

Attachment: This file demonstrates the bug (Warning-locks up Windows & Mac)

Comment 2

[leger]

Adding crash keyword

Comment 3

[Alan S. Jones]

Wow, confirmed marking as NEW
this occured with the attachment on Win95 Mozilla build 6-13-00 

nothing came out on the console when this locked.

Comment 4

[Pierre Saslawsky]

With the same build on the Mac, I get the following console output. As you can 
see, it is an infinite loop in Layout. Reassigned to buster and CCd waterson.


###!!! ASSERTION: unconstrained height on totally empty line: 
'NS_UNCONSTRAINEDSIZE != aState.mAvailSpaceRect.height', file nsBlockFrame.cpp, 
line 4372

###!!! ASSERTION: bad starting offset: 'aStartingOffset <= mFrag->GetLength()', 
file nsTextTransformer.cpp, line 165

###!!! ASSERTION: redo line on totally empty line: 
'aState.IsImpactedByFloater()', file nsBlockFrame.cpp, line 4370

###!!! ASSERTION: unconstrained height on totally empty line: 
'NS_UNCONSTRAINEDSIZE != aState.mAvailSpaceRect.height', file nsBlockFrame.cpp, 
line 4372

###!!! ASSERTION: bad starting offset: 'aStartingOffset <= mFrag->GetLength()', 
file nsTextTransformer.cpp, line 165

###!!! ASSERTION: redo line on totally empty line: 
'aState.IsImpactedByFloater()', file nsBlockFrame.cpp, line 4370

###!!! ASSERTION: unconstrained height on totally empty line: 
'NS_UNCONSTRAINEDSIZE != aState.mAvailSpaceRect.height', file nsBlockFrame.cpp, 
line 4372
 

Comment 5

[Pierre Saslawsky]

Marking Platform=all because Alan Jones confirmed the problem on Win95

Comment 6

[buster]

Attachment: a more thorough test case. Shows that only open-quote; causes the problem. For some reason, strings do not.

Comment 7

[buster]

Attachment: patch containing 2 different possible fixes

Comment 8

[buster]

The patch I attached (actually, either of them, both included and commented 
out), fixes the immediate problem so the page lays out without going into an 
infinite loop.  The code loops because we make no progress getting through the 
text.  It always returns that it's not complete, so line layout keeps making 
continuing frames for it.  Whenever you see these asserts, that's (almost) 
always the case.

I'm not happy with the fix because it in no ways explains why using 
"open-quote;" breaks, but using an arbitrary string does not.  Also, I've done 
very little testing to verify that this fix is the right thing.

Assigning to Erik to review the patch and let us know if it's the right thing to 
do.  

Comment 9

[Frank Tang]

Mark nsbeta2 since it crash.

Comment 10

[leger]

Putting on [nsbeta2+] radar for fix to stop crash only.

Comment 11

[Erik van der Poel]

Shanjian, please stop the crash only (no other fixing). Let me know if you have
questions. I will be gone 7/15 - 8/27.

Comment 12

[Shanjian Li]

Because the openquote is provided from style sheet, it is in a 
separate text fragment. For pseudo class :first-letter, we are
currently go one more further if the first "letter" is not a
letter. In this case, a quotation mark. Simply extend the 
contentLen to be 2 will cause crash in this case. 

Unfortunately, there does not seem to be a simple fix to this 
problem. I propose the following plan to fix this problem:
1, In MeasureText, if first letter is not a letter, some flag
  can be set to notify the task is not finished yet.
2, In next reflow run in MeasureText, same logic applied 
  again to setup another first letter frame.

Since I could not find a easy implementation, I will just fix the 
crash and leave everything else to erik.

Comment 13

[Shanjian Li]

remove nsbeta2+ and crash keyword, since crash has been fixed.
reassign the bug back to erik.

Comment 14

[Erik van der Poel]

Since this bug concerns the crash itself, I've added the "crash" keyword again,
and restored the "[nsbeta2+]" status. I created a separate bug for the
first-letter issue:

  http://bugzilla.mozilla.org/show_bug.cgi?id=45091

Re-assigning this bug back to Shanjian. Please mark it FIXED. Thanks!

Comment 15

[Frank Tang]

w/ shanjian. The crash have been fixed. Mark this bug fixed.

Comment 16

[ckritzer (gone)]

Marking VERIFIED FIXED on:
- MacOS9 2000-07-12-15-M17 Commercial
- Linux6 2000-07-12-21-M17 Commercial
- Win98  2000-07-12-21-M17 Commercial