hangs when page with float printed

RESOLVED FIXED

Status

()

Core
Printing: Output
--
critical
RESOLVED FIXED
13 years ago
10 years ago

People

(Reporter: JaFojtik, Assigned: Eli Friedman)

Tracking

({hang, testcase})

Trunk
hang, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.6) Gecko/20050225 Firefox/1.0.1

Firefox crashes when this page is printed.

Reproducible: Always

Steps to Reproduce:
1. Open a page
2. Print it


Actual Results:  
Firefox hangs - no response

Comment 1

13 years ago
Same with 2005031005/Seamonkey-trunk/WXP in Print Preview, marking NEW.
-> Core / Printing
Assignee: firefox → printing
Severity: normal → critical
Status: UNCONFIRMED → NEW
Component: General → Printing
Ever confirmed: true
Keywords: hang
Product: Firefox → Core
QA Contact: general
Summary: Firefox crashes when this page is printed → Trunk FF101 hangs when this page is printed
Version: unspecified → Trunk

Comment 2

13 years ago
fails also Deer park alpha 2 Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.8b4) Gecko/20050715 Firefox/1.0+
Keywords: testcase

Comment 3

12 years ago
Created attachment 202582 [details]
Reduced testcase

I reduced the URL to three floats.

Comment 4

12 years ago
fails Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060612 Minefield/3.0a1

are bug 331039 and bug 255982 dups?
Summary: Trunk FF101 hangs when this page is printed → hangs when page with float printed

Comment 5

11 years ago
Mats, is this one for you? 
Bug 354713 is probably a dupe of this one.

Comment 7

11 years ago
Anyone tried this on Firefox/Linux or MacOS X?  I submitted Bug #354713, and just looking at the testcase from the Reuters site where I saw it.  It does have a few floats in it, and I was able to confirm the Reuters site will fail across Windows XP SP2, Linux (Gentoo), and MacOS X when trying to print to a real printer or file.  I'm curious to know if this my bug is a dupe or not.  I would imagine with as big of a site as Reuters, and with their poll, the issue has to have been run into way more often now.

Updated

11 years ago
Blocks: 360224
Duplicate of this bug: 379473
Duplicate of this bug: 354713

Comment 10

11 years ago
Any work towards fixing this bug?  There are a lot of workout/nutrition plans on this site I discovered this bug on that I'd like to print without the workaround hassle.

Updated

11 years ago
OS: Windows XP → All
Hardware: PC → All

Updated

11 years ago
Flags: blocking1.9?

Comment 11

11 years ago
Created attachment 267393 [details] [diff] [review]
Patch

Comment 12

11 years ago
After apply this patch, this bug and bug 360224, bug 255982, bug 382573, bug 376218, bug 331039 will not happen. There is a problem that the text beyond the page will not show up in preview or get printed. So I'd rather call this patch as a workaround. But it is much better than hang.

Updated

11 years ago
Attachment #267393 - Flags: review?(roc)
LINE_REFLOW_TRUNCATED should not be set for lines that are at the top of the page. See:

  else if (NS_FRAME_IS_TRUNCATED(frameReflowStatus)) {
    // if the frame is a placeholder and was complete but truncated (and not at the top
    // of page), the entire line will be pushed to give it another chance to not truncate.
    if (nsGkAtoms::placeholderFrame == aFrame->GetType()) {
      *aLineReflowStatus = LINE_REFLOW_TRUNCATED;
    }
 
and nsHTMLReflowState::SetTruncated.

So in theory the only part of your patch that should be needed is this one:
-      } else {
+      } else if (!aState.IsAdjacentWithTop()) {
         // There's nowhere to retry placing the line. Just treat it as if
         // we placed the float but it was truncated so we need this line
         // to go to the next page/column.
         lineReflowStatus = LINE_REFLOW_TRUNCATED;

Comment 14

11 years ago
> So in theory the only part of your patch that should be needed is this one:
> -      } else {
> +      } else if (!aState.IsAdjacentWithTop()) {
>          // There's nowhere to retry placing the line. Just treat it as if
>          // we placed the float but it was truncated so we need this line
>          // to go to the next page/column.
>          lineReflowStatus = LINE_REFLOW_TRUNCATED;
> 
Actually this is the only place that I do not confirm.
I just add it according to the comments of  PushTruncatedPlaceholderLine.

The following one prevent the hang for this bug and other wiki related bugs.

>-      if (LINE_REFLOW_TRUNCATED == lineReflowStatus) {
>+      if (LINE_REFLOW_TRUNCATED == lineReflowStatus && >!aState.IsAdjacentWithTop()) {
>         // Push the line with the truncated float 
>         PushTruncatedPlaceholderLine(aState, aLine, lastPlaceholder, >*aKeepReflowGoing);

And the following one prevent the hang for bug 255982.
>-    else { 
>+    else if (!wasAdjacentWIthTop) { 
>       // At least one float is truncated, so fix up any placeholders that got >split and 
>       // push the line. XXX It may be better to put the float on the next >line, but this 
>       // is not common enough to justify the complexity. Or maybe it is now...
 

Then I need you to explain how LINE_REFLOW_TRUNCATED is being returned here with !aState.IsAdjacentWithTop().

Comment 16

11 years ago
I have check the aState.IsAdjacentWithTop() here.
It's true.
The result is not confirm with the log mentioned.

  else if (NS_FRAME_IS_TRUNCATED(frameReflowStatus)) {
    // if the frame is a placeholder and was complete but truncated (and not at the top
    // of page), the entire line will be pushed to give it another chance to not truncate.
    if (nsGkAtoms::placeholderFrame == aFrame->GetType()) {
      *aLineReflowStatus = LINE_REFLOW_TRUNCATED;
We should assert aState.IsAdjacentWithTop there. And I'd like someone (i.e. you :-) ) to figure out why NS_FRAME_IS_TRUNCATED(frameReflowStatus) is true even though we're adjacent to the top. That seems like the real underlying bug.

Comment 18

11 years ago
  [1] nsHTMLReflowState::SetTruncated(this = 0x803d2d0, aMetrics = STRUCT, aStatus = 0x803da24), line 2252 in "nsHTMLReflowState.cpp"
  [2] nsHTMLScrollFrame::Reflow(this = 0x9725558, aPresContext = 0x8d05750, aDesiredSize = STRUCT, aReflowState = STRUCT, aStatus = 0), line 805 in "nsGfxScrollFrame.cpp"
  [3] nsBlockReflowContext::ReflowBlock(this = 0x803d240, aSpace = STRUCT, aApplyTopMargin = 1, aPrevMargin = STRUCT, aClearance = 0, aIsAdjacentWithTop = 0, aComputedOffsets = STRUCT, aFrameRS = STRUCT, aFrameReflowStatus = 0), line 370 in "nsBlockReflowContext.cpp"
  [4] nsBlockFrame::ReflowFloat(this = 0x9bba210, aState = CLASS, aPlaceholder = 0x8c56e50, aFloatCache = 0x97f4c80, aReflowStatus = 0), line 5302 in "nsBlockFrame.cpp"
  [5] nsBlockReflowState::FlowAndPlaceFloat(this = 0x803e548, aFloatCache = 0x97f4c80, aIsLeftFloat = 0x803d674, aReflowStatus = 0, aForceFit = 0), line 738 in "nsBlockReflowState.cpp"
  [6] nsBlockReflowState::AddFloat(this = 0x803e548, aLineLayout = CLASS, aPlaceholder = 0x8c56e50, aInitialReflow = 0, aReflowStatus = 0), line 546 in "nsBlockReflowState.cpp"
  [7] nsLineLayout::AddFloat(this = 0x803db5c, aFrame = 0x8c56e50, aReflowStatus = 0), line 255 in "nsLineLayout.h"
  [8] nsLineLayout::ReflowFrame(this = 0x803db5c, aFrame = 0x8c56e50, aReflowStatus = 0, aMetrics = (nil), aPushedFrame = 0), line 923 in "nsLineLayout.cpp"
=>[9] nsBlockFrame::ReflowInlineFrame(this = 0x9bba210, aState = CLASS, aLineLayout = CLASS, aLine = CLASS, aFrame = 0x8c56e50, aLineReflowStatus = 0x803dad0), line 3434 in "nsBlockFrame.cpp"

This is the stack when frameReflowStatus is set to NS_FRAME_TRUNCATED.
At this time mFlags.mIsTopOfPage is false.
And I see the comments here.
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsBlockReflowState.cpp#705
Here mY changed, which makes the IsAdjacentWithTop false.
Okay, I see. Then we need to find a way to make IsAdjacentWithTop return true as long as we are placing floats at the beginning of the first line, even if some of those floats have been placed and moved mY down.

Comment 20

11 years ago
Roc, I don't know how to do this.
Reset mY before ReflowFloat?
Or pass a "WasAdjacentWithTop" to ReflowFloat?

(In reply to comment #19)
> Okay, I see. Then we need to find a way to make IsAdjacentWithTop return true
> as long as we are placing floats at the beginning of the first line, even if
> some of those floats have been placed and moved mY down.
> 

I think instead of IsAdjacentWithTop testing mY, it should test whether there is a non-empty line before the current line. Roughly what this does:
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsBlockFrame.cpp#2578

But the easiest way to do it might be to have a flag to indicate whether a non-empty line has been placed. That flag could be updated where we currently update mY (but not if we're updating mY just because we are placing a float and moving to the next band).

Comment 22

11 years ago
Roc, I am not familiar with that part of code.
Can you take over this bug?
ok
(Is there any reason that the variable is called "wasAdjacentWIthTop" instead of "wasAdjacentWithTop"?)

Updated

11 years ago
Blocks: 323652
ain't autcomplete great?
(Assignee)

Comment 26

11 years ago
Taking; I think I've got a pretty good handle on this.
Assignee: printing → sharparrow1
(Assignee)

Comment 27

11 years ago
Created attachment 269750 [details] [diff] [review]
Patch v1

This patch fixes a some of the hangs, including this bug, bug 323652, bug 255982, and both testcases in bug 331039.  It doesn't fix bug 350684 or bug 360224; however, as far as I can tell, those aren't float bugs; something odd is triggering in text frame reflow (even though suppressInitialBreak is getting passed to BreakAndMeasureText, it's saying that 0 characters fit).

This approach isn't especially clean, but it basically works, and it's relatively clear what's going on.  Any suggestions?
Attachment #267393 - Attachment is obsolete: true
Attachment #269750 - Flags: review?(roc)
Attachment #267393 - Flags: review?(roc)
Comment on attachment 269750 [details] [diff] [review]
Patch v1

Looks good...

+ if (placed && (forceFit || !NS_FRAME_IS_TRUNCATED(aReflowStatus))) {

I think it would be clearer if you wrote this as
  if (forceFit || (placed && !NS_FRAME... 

+      if (IsAdjacentWithTop()) {
+        aReflowStatus = NS_INLINE_LINE_BREAK_BEFORE();

Need a comment here pointing out that forceFit is false, therefore aLineLayout.LineIsBreakable() must be true ... possibly with an asertion of
aLineLayout.LineIsBreakable().
Attachment #269750 - Flags: superreview+
Attachment #269750 - Flags: review?(roc)
Attachment #269750 - Flags: review+
(Assignee)

Comment 29

11 years ago
Created attachment 269959 [details] [diff] [review]
Final patch
Attachment #269750 - Attachment is obsolete: true
(Assignee)

Comment 30

11 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
      NS_WARN_IF_FALSE(NS_FRAME_IS_TRUNCATED(reflowStatus) && aForceFit,
                       "This situation currently leads to data not printing");

This fires when the frame is complete. Should we add "&& NS_FRAME_IS_NOT_COMPLETE(reflowStatus)"?
(Assignee)

Comment 32

11 years ago
Created attachment 270973 [details] [diff] [review]
Warning fix

Yeah, I messed up that warning...
Attachment #270973 - Flags: review?(roc)
Attachment #270973 - Flags: superreview+
Attachment #270973 - Flags: review?(roc)
Attachment #270973 - Flags: review+

Updated

11 years ago
Duplicate of this bug: 360224

Updated

11 years ago
No longer blocks: 360224

Updated

11 years ago
Duplicate of this bug: 331039

Updated

11 years ago
Duplicate of this bug: 361821

Updated

11 years ago
Duplicate of this bug: 389553

Updated

11 years ago
Duplicate of this bug: 392439
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; it; rv:1.8.1.7) Gecko/20070914 Firefox/2.0.0.7
Build Identifier: 2007091417

Firefox crashes when this page is printed: http://it.wikipedia.org/wiki/Sistema_di_numerazione_arabo

Reproducible: Always

Steps to Reproduce:
1. Open the page
2. Print it


Actual Results:  
Firefox hangs - no response
(Assignee)

Comment 39

10 years ago
We know... it will be fixed in Firefox 3.
Thanks for your answer Eli. 

Anyway I would inform you that the issue I described above is still presente in Gran paradiso a8: http://releases.mozilla.org/pub/mozilla.org/firefox/releases/granparadiso/alpha8/win32/en-US/Gran%20Paradiso%20Setup%20Alpha%208.exe

Moreover I would let you know i have filled bug 397528 referring to this specific topic related to wikipedia.

Comment 41

10 years ago
When I came across this problem, I tracked it to the CSS used by Wikipedia.  The problem is an interaction between 'overflow: hidden' and 'clear: right'.  In Wikipedia's style sheet, you will find these two class definitions.

div.thumb {
    margin-bottom: 0.5em;
    border-style: solid;
    border-color: White;
    width: auto;
    overflow: hidden;
}
div.tright {
    float: right;
    clear: right;
    border-width: 0.5em 0 0.8em 1.4em;
}

If either the overflow or clear properties are commented out in the css code,
the page prints (and previews).  With both properties present, Firefox hangs.  The reason so many pages do not exhibit this behavior is that many pages do not actually use both these classes.  If they are both used on a page, it will hang.  

The code to fix the bug has been written.  The fact that it isn't in any particular alpha release doesn't bother me.  It IS in the 3.0a9pre code so it WILL be in the 3.0 release (as long as there are no problems with it).  Just give it time.

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

Updated

10 years ago
Blocks: 255982

Updated

10 years ago
Flags: blocking1.9?

Updated

10 years ago
Duplicate of this bug: 398376

Updated

10 years ago
Duplicate of this bug: 423827
You need to log in before you can comment on or make changes to this bug.