Closed Bug 909078 Opened 8 years ago Closed 8 years ago

Browser hangs attempting to print preview on certain page

Categories

(Core :: Print Preview, defect)

19 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla26
Tracking Status
firefox22 --- wontfix
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- fixed
firefox26 --- fixed
firefox-esr17 --- unaffected
firefox-esr24 26+ verified

People

(Reporter: alice0775, Assigned: mats)

References

Details

(Keywords: hang, regression, testcase)

Attachments

(2 files, 1 obsolete file)

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/17143a9a0d83
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130824030203

Steps To Reproduce:
1. Open http://en.wikipedia.org/wiki/H-II_Transfer_Vehicle
2. Print Preview (Alt > File > Print Preview)

Actual Results:
Browser hang up


Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/e0d7b394462b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121108022104
Bad:
http://hg.mozilla.org/mozilla-central/rev/27ae6c86237f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121108165332
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e0d7b394462b&tochange=27ae6c86237f

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/fdcb6474f4b4
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121108080032
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/ac1fdaa261f2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121108081030
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fdcb6474f4b4&tochange=ac1fdaa261f2

Suspected :Bug 685012
Keywords: hang
Duplicate of this bug: 909074
Attached file testcase html
Keywords: testcase
STR
1. Open attachment 795140 [details]
2. Print Preview (Alt > File > Print Preview)
3  Set scale 200% if necessary
   
In local build
Last Good: dbf63c3f34e9 
First Bad: ea3dd343ee5e

Triggered by: ea3dd343ee5e	Mats Palmgren — Bug 685012 - Implement page-break-inside:avoid in layout. r=fantasai,sr=roc
Attached patch fix+tests (obsolete) — Splinter Review
What happens is that the inner table doesn't fit, and since the <caption>
caused the top-of-page flag for the inner table reflow state to be false,
it thinks it's OK to request a forced page break before it even though
the outer table frame is top-of-page.

The fix is to check the outer table frame's reflow state for top-of-page.
I'm doing that rather than lying and making the inner top-of-page flag
be a copy of the outer to avoid affecting other stuff -- it's just
break-before that is special, since it really is for the whole <table>
including the <caption>.

https://tbpl.mozilla.org/?tree=Try&rev=84b655588269
Assignee: nobody → matspal
Attachment #795163 - Flags: review?(roc)
Comment on attachment 795163 [details] [diff] [review]
fix+tests

Hmm, now that I think about it, the exact same thing probably happens
when the break-before comes from descendants as well...
Attachment #795163 - Attachment is obsolete: true
Attachment #795163 - Flags: review?(roc)
Attached patch fix+tests, v2Splinter Review
Actually, I changed my mind about pretending the inner table frame is
top-of-page.  After investigating how we use the mIsTopOfPage flag it's
clear that is already used as a "logical" flag that really should be used
only for deciding on page breaks.  I was initially worried it would affect
applying top margins, but we use a separate IsAdjacentWithTop() method for
that which is "physical" (based on the distance from the edge).
http://hg.mozilla.org/mozilla-central/annotate/60c382ba1773/layout/generic/nsBlockFrame.cpp#l2753

So, let's make the inner table frame be top-of-page when the outer table
frame is (by removing the the code clears the flag when there's a top-side
caption -- the value is already a copy of the outer state at this point).

https://tbpl.mozilla.org/?tree=Try&rev=d5c161812642
Attachment #795235 - Flags: review?(roc)
Comment on attachment 795235 [details] [diff] [review]
fix+tests, v2

Review of attachment 795235 [details] [diff] [review]:
-----------------------------------------------------------------

avoid-8-ref.html is missing.
Attachment #795235 - Flags: review?(roc) → review+
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7)
> avoid-8-ref.html is missing.

I'm intentionally re-using the avoid-6-ref.html since the result is the same.
https://hg.mozilla.org/integration/mozilla-inbound/rev/05670d699bdc
Flags: in-testsuite+
OS: Windows 7 → All
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/05670d699bdc
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Is this possible to uplift the patch to fix in Firefox24 and 24esr?
Flags: needinfo?(matspal)
I think the patch would apply cleanly, but I don't think it meets the
branch criteria for 24.  Aurora perhaps...
Flags: needinfo?(matspal)
Comment on attachment 795235 [details] [diff] [review]
fix+tests, v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 685012
User impact if declined: hang in Print Preview on pages that use page-break-inside:avoid under rare conditions
Testing completed (on m-c, etc.): on m-c since 2013-08-26
Risk to taking this patch (and alternatives if risky): low, but the change affects all paginated layout, not just page-break-inside:avoid (which is rare) so the risk/reward doesn't seem very attractive to me.  Should be good for Aurora though, to get wider testing.
String or IDL/UUID changes made by this patch: none
Attachment #795235 - Flags: approval-mozilla-aurora?
Too late to take for FF24, but we can take in our first ESR update (24.1). I'm setting tracking-firefox24 to ? until we have a tracking-firefox-esr24 flag.
Attachment #795235 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 795235 [details] [diff] [review]
fix+tests, v2

[Approval Request Comment]
see comment 13
Attachment #795235 - Flags: approval-mozilla-esr24?
Attachment #795235 - Flags: approval-mozilla-esr24? → approval-mozilla-esr24+
Verified as fixed with latest ESR 24 nightly build from 2013-12-04, on: Ubuntu 13.10 32-bit, Win 7 64-bit, and Mac OS X 10.9, using both the STR from comment 0 and comment 3.

Note: on Mac, I've used the Open PDF in Preview option to test this.
QA Contact: manuela.muntean
You need to log in before you can comment on or make changes to this bug.