Closed Bug 266225 Opened 20 years ago Closed 20 years ago

Crash [@ nsFieldSetFrame::Reflow ]

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Platform:
HP
Windows XP
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: robert.strong.bugs, Unassigned)

Details

(4 keywords)

Crash Data

Attachments

(2 files)

Testcase (causes crash)
118 bytes, text/html
Details
patch
1.41 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
asa
: approval-aviary+
mkaply
: approval1.7.5-
mkaply
: approval1.7.6+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041025
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041025

The soon to be attached simplified testcase causes a crash @
nsFieldSetFrame::Reflow. TB1542262Z

Reproducible: Always
Steps to Reproduce:
1. Open testcase
2.
3.

Actual Results:  
Crash

Expected Results:  
No crash

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB1542262Z
Stack Signature	 nsFieldSetFrame::Reflow 820c5d62
Source File, Line No.
c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/layout/html/forms/src/nsFieldSetFrame.cpp,
line 381 

Note: This also affects the latest Firefox branch though I didn't send a
talkback for it.
Testcase contains the following:
<HTML>
<HEAD>
</HEAD>
<BODY>
<FIELDSET STYLE="float:right; text-indent:999px;">Test</FIELDSET>
</BODY>
</HTML>
Adding keywords crash and testcase
Keywords: crash, testcase
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041025
WFM
Hardware: PC → HP
it crashes at
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/html/forms/src/nsFieldSetFrame.cpp&mark=375&rev=#370
with mLegendFrame being nsNull.
Assignee: general → nobody
Component: Browser-General → Layout: Form Controls
QA Contact: general → core.layout.form-controls
taking
Assignee: nobody → bernd_mozilla
Attached patch patch — — Splinter Review 

    
  

        Attachment #163523 -
        Flags: superreview?(bzbarsky)

        Attachment #163523 -
        Flags: review?(bzbarsky)

    
    

    
  
Comment on attachment 163523 [details] [diff] [review]
patch

r+sr=bzbarsky

        Attachment #163523 -
        Flags: superreview?(bzbarsky)

        Attachment #163523 -
        Flags: superreview+

        Attachment #163523 -
        Flags: review?(bzbarsky)

        Attachment #163523 -
        Flags: review+

    
    

    
  
Comment on attachment 163523 [details] [diff] [review]
patch

the fix is small and low risk i think it should go on branch

        Attachment #163523 -
        Flags: approval1.7.x?

        Attachment #163523 -
        Flags: approval-aviary?

    
    

    
  
Comment on attachment 163523 [details] [diff] [review]
patch

a=mkaply for 1.7.

Please send a note to aviary for aviary changes this late in the game.

        Attachment #163523 -
        Flags: approval-aviary? → approval-aviary+

    
  

        Attachment #163523 -
        Flags: approval1.7.x?

        Attachment #163523 -
        Flags: approval1.7.x+

        Attachment #163523 -
        Flags: approval-aviary?

        Attachment #163523 -
        Flags: approval-aviary+

    
    

    
  
fixed on 1.7x the aviary decision is open to the aviary people, maybe it should
go in after 1.0, so that it will be in 1.0.1
Keywords: fixed1.7.x

    
    

    
  
Verifying fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.4)
Gecko/20041028

Still crashes with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5)
Gecko/20041028 which is to be expected since the patch has only been checked
into 1.7x

    
    

    
  
Thank you Stephen. I have all the testcases stored as data:text/html so I can
verify the unreduced testcase as well. I must have grabbed the wrong one and the
patch does indeed fix this with 20041028 Trunk.

    
    

    
  
Comment on attachment 163523 [details] [diff] [review]
patch

dbaron says we should take this pending another quick review of the c++ order
of operations by him or brendan. Please land when that review happens.

        Attachment #163523 -
        Flags: approval-aviary? → approval-aviary+

    
    

    
  
the patch did not make it for 1.0 so closing this bug as 
"There is currently no scheduled Firefox post 1.0 work scheduled for the branch"
quote from tinderbox.

    
    

    
  
.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED

    
    

    
  
Verified FIXED using the testcase
https://bugzilla.mozilla.org/attachment.cgi?id=163516&action=view on build
2004-11-15-05 on Windows XP.
Status: RESOLVED → VERIFIED

    
    

    
  
We need to back this out because this is something web authors don't have in
Firefox and in our efforts to make these two Geckos compatible, we need to be
crash for crash compatible here. 

Bernd, can you pull this for us, please?

    
    

    
  
(In reply to comment #18)
> We need to back this out because this is something web authors don't have in
> Firefox and in our efforts to make these two Geckos compatible, we need to be
> crash for crash compatible here. 
Sorry for the spam... this implies that future fixes or at least a subset of
these fixes involving crashes of this nature will not be applied to the Trunk at
least until some time in the future. Is this true and if it is then what is the
time frame?

    
    

    
  
No, I am not going to take part in this, I work hard to get this lizzard stable
asking for patch that makes 1.7.5 deliberately crash is too much for me. If you
want to back this out go and find somebody else who wants to checkin a fix that
makes the lizzard crash.

    
    

    
  
Sorry bernd, but I think removing this from the 1.7 branch is the right thing to
do. We'll find someone else to do the dirty work.

    
    

    
  
Roc, Asa;  

   May I at least request this patch be applied to the aviary and 1.7 branches
shortly after 1.7.5 is released, if nothing else I would definately prefer to
have this crasher fixed on the actual code-tree's, just in case another release
of either of these branches happens.

    
    

    
  
Comment on attachment 163523 [details] [diff] [review]
patch

Backed out of 1.7.5. I'll get this on 1.7.6 as soon as 1.7.5 ships.

        Attachment #163523 -
        Flags: approval1.7.6+

        Attachment #163523 -
        Flags: approval1.7.5-

        Attachment #163523 -
        Flags: approval1.7.5+

    
    

    
  
Per comment #24
Flags: blocking1.7.6?

    
    

    
  
the patch is again in
Keywords: fixed1.7.5 → fixed1.7.6
Flags: blocking1.7.6? → blocking1.7.6+

    
    

    
  
It seems this bug _was_ fixed for 1.7.6 on Christmas Eve day. The blocking1.7.6+
flag is not necessary, anymore. 
Status: VERIFIED → REOPENED
Resolution: FIXED → ---

    
    

    
  
As I stated in comment 26 I checked the patch in again, but maybee its time to
back it out again as we don't crash enough in the suite. Reassigning the bug, to
be decoupled from mozilla politics, that I am not interested in.
Assignee: bernd_mozilla → nobody
Status: REOPENED → NEW

    
    

    
  
Adding this to the nominations radar.  There is quite unfortunately, an
interesting story to this bug.  This is currently checked in on 1.7.6, but not
aviary.

See comment 14, comment 18, comment 24, etc.
Flags: blocking-aviary1.0.1?

    
    

    
  
checked in on AVIARY_1_0_1_20050124_BRANCH
Keywords: fixed-aviary1.0.1

    
    

    
  
Already in, setting blocking flag to get off nominations radar
Flags: blocking-aviary1.0.1? → blocking-aviary1.0.1+

    
    

    
  
Shouldn't this bug be closed fixed? This was checked into the trunk long ago
(comment 12 and 13). I don't understand why piskozub reopened it, next time add
more explicit comments if you did it on purpose.
Status: NEW → RESOLVED
Closed: 20 years ago → 20 years ago
Resolution: --- → FIXED

    
    

    
  
Sorry. I believe bugzilla did the actual reopening. I only wanted to comment
that blocking1.7.6+ is no longer needed. As I do not receive emails with my own
changes (seems stupid to do so), I had no idea it has been reopened.

Thanks for catching it. Verifying, od course.
Status: RESOLVED → VERIFIED

    
    

    
  
layout/forms/crashtests/266225-1.html
http://hg.mozilla.org/mozilla-central/rev/b0337b6287f3
Flags: in-testsuite+
Crash Signature: [@ nsFieldSetFrame::Reflow ]

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: