crash when attempting to print the page [@ nsCachedStyleData::GetStyleData]

VERIFIED FIXED

Status

()

Core
Layout: Tables
--
critical
VERIFIED FIXED
13 years ago
7 years ago

People

(Reporter: Dave Swegen, Assigned: Bernd)

Tracking

({crash, verified1.8.0.7, verified1.8.1})

Trunk
x86
Linux
crash, verified1.8.0.7, verified1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.7.10) Gecko/20050717 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.7.10) Gecko/20050717 Firefox/1.0.6

Trying to print the page in the URL field causes firefox to crash. 

Reproducible: Always

Steps to Reproduce:
1. Go to the page in the URL field.
2. Print using the dialog in File->Print
3. Watch firefox crash

Actual Results:  
Print progress dialog box comes up briefly, then firefox crashes.

Expected Results:  
Page should be printed, and functionality return to normal.

The interesting thing is that from this [1] list of geocaches, only the one in
the URL field crashes. All the others print quite happily.

[1] http://www.geocaching.com/seek/nearest.aspx?lat=55.8815&lon=14.2328

Updated

13 years ago
Keywords: crash

Comment 1

13 years ago
Please provide a talkback ID for the crash. http://kb.mozillazine.org/Talkback

Comment 2

13 years ago
Also crashing while printing this page.

http://wiki.jboss.org/wiki/Wiki.jsp?page=IWantToConnectToXYZShouldIUseJCA

Comment 3

13 years ago
I have encountered the same problem with many of the longer articles on Wikipedia.
Last time it happened it was 

http://de.wikipedia.org/wiki/T%C3%BCrkei (German page on Turkey)

but it happens frequently and I will add more URLs as they come.


Kind regards


zapyon

Comment 4

13 years ago
Sorry, I forgot. I am using Firefox 

Mozilla/5.0 (X11; U; Linux i686; de-DE; rv:1.7.10) Gecko/20050825 Firefox/1.0.4
(Debian package 1.0.4-2sarge3)

or Mozilla

Mozilla/5.0 (X11; U; Linux i686; de-AT; rv:1.7.8) Gecko/20050831
Debian/1.7.8-1sarge2

on Debian Linux.


Kind regards


Andreas


Comment 6

13 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8) Gecko/20051025 Firefox/1.5

I confirm such a crash on
http://travel.state.gov/visa/reciprocity/Country%20Folder/I/Iran.htm

After two crashes, I saved the page, and printing that document also crashed Firefox.

Comment 7

13 years ago
http://www.ariba.com/suppliers/suppliers_FAQ.cfm

THe above link also crashes firefox (the program hangs in the "preparing to print" step). Quality feedback agent does not pop up.

Comment 8

13 years ago
Robert, what build were you using? You guys need to get stacktraces or talkback ID's for the crashes. See comment 1 and comment 5.

Comment 9

13 years ago
I discovered this problem a while back. After getting input from others, I've come to learn that there's something in the CSS code that is causing the crash. Honestly, I don't know what that problem is.

It is, though, because of CSS. Try disabling CSS before printing again.

Comment 10

13 years ago
I have observed this problem when trying to print to a network printer with both postscript and non-postscript drivers.  It doesn't crash all the time, but enough to be annoying. Crash is repeatable for a given web page that is crashing.  Problem has been present with all pre-release versions of Firefox 1.5.  Firefox 1.x.x didn't have this problem. 

Also could not successfully roll back to version 1.0.7. to work around this printing problem. There is a large blank grey area at bottom of the window after reinstalling 1.0.7.  Had to revert to IE to print.

Comment 11

13 years ago
Additional web page on which to reproduce:

http://news.yahoo.com/comics/uclickcomics/20051218/cx_db_uc/db20051218;_ylt=ApCFIiEg2aY2sBTWKpEgJgPV.i8C;_ylu=X3oDMTBiMW04NW9mBHNlYwMlJVRPUCUl

I did this when attempting to print the first sheet of this page from within Print Preview, with the page being one of multiple tabs, with the printer being a color printer.  (Not sure if any of this makes a difference, but just in case.)  Worked on 1st attempt; hung with no crash on 2nd attempt; crashed on next 3 attempts, of which one (maybe 2) managed to get something to the printer.

I can't find a Talkback ID for any of this right now because the server module that looks for these is not working (keeps giving an internal server error).

Updated

13 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Crash when attempting to print the page → crash when attempting to print the page [@ nsCachedStyleData::GetStyleData]
Version: unspecified → Trunk

Updated

13 years ago
Component: General → Layout: Tables
Product: Firefox → Core
QA Contact: general → layout.tables

Comment 12

13 years ago
Created attachment 206359 [details]
stacktrace
(Assignee)

Comment 13

13 years ago
we (I) need a reduced testcase here.
Keywords: qawanted
(Assignee)

Comment 14

13 years ago
Created attachment 206869 [details]
testcase

not really reduced, but reproducible, so it seems we have this since FF1.0?
(Assignee)

Comment 15

13 years ago
             tblO 034CBF34 d=5970,14421 me=5040 status=0x11
             block 034DE7B0 d=5970,14421 me=5040 status=0x13
            block 034CBB94 d=5970,284 me=1111 status=0x3 <<< where is all the height gone?
           cell 034CBB34 d=6000,300 me=1155 status=0x3
(Assignee)

Comment 16

13 years ago
Bug 292124 will make the crash with the testcase go away. However the underlaying problem does not vanish. The thing here seems to me like:

The table gets a constrained availHeight. It then does not fit but split and creates nextinflows. We might hit the end of
ReflowBlockFrame

      else {
          // Push the line that didn't fit and any lines that follow it
          // to our next-in-flow.
          UndoSplitPlaceholders(aState, lastPlaceholder);
          PushLines(aState, aLine.prev());
          aState.mReflowStatus = NS_FRAME_NOT_COMPLETE;
        }
      }

If this happens the height gets reduced as shown in the reflow above, My feeling is that we do not completely rewind the nextinflow stuff that the inner frames already did and on the next page we pull the wrong overflow frames and crash.

I am not certain that fixing the table splitting is good enough.

This raises for me the key question:
1. if a frame can split somehow, but will not fit anyway, should it split or not?

What I have in mind is atable rowgroup, where the first cell (first row) has a large nonsplittable content, so it can't be splitted, but the rows after it should go of course on the next page.

2. if 1. is answered with: do not split, shouldn't we assert.
Boris, Robert any insight on this?
(Assignee)

Updated

13 years ago
Depends on: 292124
(In reply to comment #16)
>       else {
>           // Push the line that didn't fit and any lines that follow it
>           // to our next-in-flow.
>           UndoSplitPlaceholders(aState, lastPlaceholder);
>           PushLines(aState, aLine.prev());
>           aState.mReflowStatus = NS_FRAME_NOT_COMPLETE;
>         }
>       }
> 
> If this happens the height gets reduced as shown in the reflow above, My
> feeling is that we do not completely rewind the nextinflow stuff that the
> inner frames already did and on the next page we pull the wrong overflow
> frames and crash.

I kinda got lost here. So you're saying there's a block problem here independent of tables?

> This raises for me the key question:
> 1. if a frame can split somehow, but will not fit anyway, should it split or
> not?
> 
> What I have in mind is atable rowgroup, where the first cell (first row) has a
> large nonsplittable content, so it can't be splitted, but the rows after it
> should go of course on the next page.

In general it should split. The first part, which doesn't fit, will be placed and truncated. The rest will move to the next column or page.
(Assignee)

Comment 18

13 years ago
>I kinda got lost here. So you're saying there's a block problem here
independent of tables?

I am not sure that its a block problem, but more a structural problem (aka my lack of understanding):

assume we have

<A>
 <B>
 <C>
 <D>
</A>

after lets assume that we need to split  <C> as H_b + H_c >  h_a_available 

<P1>
 <A>
  <B>
  <C1>  (C did create a next inflow C2, who has the reminder)
  at this place h_B + h_C1 are still larger than the available height for h_a.
  so it would be sensable to break before <C1> as h_b < h_a_available
  so we need to push <C1> too, we should then undo the splitting and push not              
  not only <C1>  but <C> and remove <C2> from the overflow list.

so at the end we have

<P>
 <A>
  <B>
 </A>
</P1>
<P2>
 <A2>
  <C1>
 </A2>
</P2>
<P3>
 <A3>
  <C2>
  <D>
 </A3>
</P3>

And my feeling is that the unwinding described above might have issues.
(In reply to comment #18)
That's exactly what our code is supposed to do. Except for one thing: we don't undo the split immediately. 'A' will just push both C1 and C2 to its overflow list; when C1 eventually gets reflowed and fits, then it will suck content from its next-in-flow(s) including C2 as long as that content fits*. If all that content fits, then C1 returns status 'complete' and its parent will delete its empty next in flow frames.

* E.g. for blocks:
http://lxr.mozilla.org/mozilla/source/layout/generic/nsBlockFrame.cpp#2411
(Assignee)

Comment 20

13 years ago
I am not sure that learning pagination with a crash bug is the way to do it, but here it goes.

Starting again from the reflow log  
>              tblO 034CBF34 d=5970,14421 me=5040 status=0x11
>             block 034DE7B0 d=5970,14421 me=5040 status=0x13
>            block 034CBB94 d=5970,284 me=1111 status=0x3 <<< where is all the height gone?
>           cell 034CBB34 d=6000,300 me=1155 status=0x3

The table has created a continuation of the row group (lets name it  C2) and keeps C1 as before. The block sees that it could probably layout the table on a next page, and pushes it to the next page. 

On the next page  we reflow the table and it has C1 and pulls C2 from the overflow list. Now it reflows C1 which will fit as it did fit allready on the first page only the addition of the caption made the outer table frame exceeding the available height. At the end of the rowgroup reflow, the row group will be marked complete (this might be allready the error) and the nextinflow will be deleted. This deletes C2, the table has reference to C2 in the rowgroups array and will try to reflow it at the next step, reflowing deleted frames dies as usually in the style system.

I think what should happen is that C1's reflow should pull all rows out of C2, leaving C2 empty. Then C1 returns saying it's complete. Its parent (nsTableFrame in this case) should then delete C2, removing C2 from its child list and not try to reflow C2 again.
So I think the problem is simply that when nsTableFrame::ReflowChildren calls nsContainerFrame::ReflowChild on C1, which empties out C2 and then deletes it
http://lxr.mozilla.org/mozilla/source/layout/generic/nsContainerFrame.cpp#890
we also need to remove C2 from nsTableFrame::ReflowChildren's rowgroup array.
(Assignee)

Comment 23

13 years ago
>nsTableFrame::ReflowChildren calls nsContainerFrame::ReflowChild on C1, which empties out C2

This is not exactly what is happening. Troy allready in 1999 found that this is to much bloat and implemented http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/tables&command=DIFF_FRAMESET&file=nsTableRowGroupFrame.cpp&rev2=3.134&rev1=3.133

in bug 82401 http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsTableRowGroupFrame.cpp&branch=3.349&root=/cvsroot&subdir=mozilla/layout/tables&command=DIFF_FRAMESET&rev1=3.251&rev2=3.252
Chris removed that pulling at all.

I am very  tempted to follow troys path 

Index: nsTableRowGroupFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/tables/nsTableRowGroupFrame.cpp,v
retrieving revision 3.349
diff -u -8 -p -r3.349 nsTableRowGroupFrame.cpp
--- nsTableRowGroupFrame.cpp	20 Nov 2005 22:05:15 -0000	3.349
+++ nsTableRowGroupFrame.cpp	13 Jan 2006 18:46:06 -0000
@@ -1269,16 +1271,19 @@ nsTableRowGroupFrame::Reflow(nsPresConte
       // Nope, find a place to split the row group 
       PRBool specialReflow = (PRBool)aReflowState.mFlags.mSpecialHeightReflow;
       ((nsHTMLReflowState::ReflowStateFlags&)aReflowState.mFlags).mSpecialHeightReflow = PR_FALSE;
 
       SplitRowGroup(aPresContext, aDesiredSize, aReflowState, tableFrame, aStatus);
 
       ((nsHTMLReflowState::ReflowStateFlags&)aReflowState.mFlags).mSpecialHeightReflow = specialReflow;
     }
+    if (mNextInFlow) {
+      aStatus = NS_FRAME_NOT_COMPLETE;
+    }
   }
   SetHasStyleHeight((NS_UNCONSTRAINEDSIZE != aReflowState.mComputedHeight) &&
                     (aReflowState.mComputedHeight > 0)); 
   
   if (aReflowState.mFlags.mSpecialHeightReflow) {
     SetNeedSpecialReflow(PR_FALSE);
   }

and to add a addtional protection layer that avoids the reference to allready deleted nextinflows in the rowgroup array.
(Assignee)

Comment 24

13 years ago
Just to make sure that my point of view is correctly visible:
<grin>this is to much bloat and implemented</grin>

and I am sorry that I did not find when 

// XXX Check aStatus to see if the frame is complete...   

this line from troys checkin got removed.
(In reply to comment #23)
> >nsTableFrame::ReflowChildren calls nsContainerFrame::ReflowChild on C1,
> >which empties out C2
> 
> This is not exactly what is happening. Troy allready in 1999 found that this
> is to much bloat and implemented
> http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/tables&command=DIFF_FRAMESET&file=nsTableRowGroupFrame.cpp&rev2=3.134&rev1=3.133

Those are in incremental reflow ... are we doing IRs here?

Anyway, those are not correct. The frame is complete when all its content fits, even if there is still a next in flow frame. In this incremental reflow situation, the frame can check that it is complete by checking that a) its list of overflowing children is empty and b) all next-in-flows have no children.
(Assignee)

Comment 26

13 years ago
Created attachment 209201 [details] [diff] [review]
patch

Do exactly what Chris told to do allready in 2001...
Attachment #209201 - Flags: superreview?(roc)
Attachment #209201 - Flags: review?(roc)
(Assignee)

Updated

13 years ago
Assignee: nobody → bernd_mozilla
I think I'd prefer a safer patch that always recreates the row group list if kidFrame had any next-in-flow before we reflowed it. comparing the pointers this way doesn't feel good to me because the next-in-flow might just possibly have been destroyed and then recreated at the same address.

Also, do you want to file a followup bug about removing the bogus status-setting like
http://lxr.mozilla.org/seamonkey/source/layout/tables/nsTableRowGroupFrame.cpp#1486
?
(Assignee)

Comment 28

13 years ago
Created attachment 209600 [details] [diff] [review]
reorder more frequently
Attachment #209201 - Attachment is obsolete: true
Attachment #209600 - Flags: superreview?(roc)
Attachment #209600 - Flags: review?(roc)
Attachment #209201 - Flags: superreview?(roc)
Attachment #209201 - Flags: review?(roc)
(Assignee)

Comment 29

13 years ago
I filed bug 324683 as the followup.
Comment on attachment 209600 [details] [diff] [review]
reorder more frequently

That's OK but it would be slightly nicer if instead of keeping kidNextInFlow, you had a PRBool hadNextInFlow = kidFrame->GetNextInFlow() != nsnull; ... that wy you don't have to keep around a dangling pointer.
Attachment #209600 - Flags: superreview?(roc)
Attachment #209600 - Flags: superreview+
Attachment #209600 - Flags: review?(roc)
Attachment #209600 - Flags: review+
(Assignee)

Comment 31

13 years ago
fix checked in
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Blocks: 336427
Is this patch something that can be considered for the 1.8.1 branch?
It doesn't seem to have caused any regressions and it fixes this crash and the crash in bug 336427 (which is supposed to be a topcrash bug).
Keywords: qawanted
Flags: blocking1.8.1?
(Assignee)

Updated

12 years ago
Attachment #209600 - Flags: approval1.8.1?
Comment on attachment 209600 [details] [diff] [review]
reorder more frequently

a=dbaron on behalf of drivers.  Please land on MOZILLA_1_8_BRANCH and mark fixed1.8.1 once you have.
Attachment #209600 - Flags: approval1.8.1? → approval1.8.1+
Flags: blocking1.8.1? → blocking1.8.1+
(Assignee)

Comment 34

12 years ago
Created attachment 231890 [details] [diff] [review]
patch against MOZILLA_1_8_BRANCH
(Assignee)

Comment 35

12 years ago
fixed on 1.8.1
Keywords: fixed1.8.1
verified on Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1b1) Gecko/20060813 BonEcho/2.0b1

Can anyone verify this on Linux ?

Comment 37

12 years ago
Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b1) Gecko/20060803 BonEcho/2.0b1. 

I no longer crash when print previewing the site in the URL of this bug. Thanks for the help, Tomcat.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
This may or may not be part of the patch for bug 346980 (see bug 346980 comment 7). Nominating for 1.8.0.x separately to make sure it gets covered.
Flags: blocking1.8.0.8?
when printing https://bugzilla.mozilla.org/attachment.cgi?id=206869&action=view the browser should not crash.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.7pre) Gecko/20060829 Firefox/1.5.0.7pre

verified 1.8.0.7
Keywords: fixed1.8.0.7 → verified1.8.0.7
Restoring lost blocking flag
Flags: blocking1.8.0.9?
(Assignee)

Comment 42

12 years ago
This got already checked in into 1.8.0.7 (http://lxr.mozilla.org/mozilla1.8.0/source/layout/tables/nsTableFrame.cpp#3228) no need to block for 1.8.0.9

Updated

12 years ago
Flags: blocking1.8.0.9?
Crash Signature: [@ nsCachedStyleData::GetStyleData]
You need to log in before you can comment on or make changes to this bug.