Browser hangs attempting to print preview on certain page

RESOLVED FIXED in Firefox 25

Status

()

Core
Print Preview
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Alice0775 White, Assigned: mats)

Tracking

({hang, regression, testcase})

19 Branch
mozilla26
hang, regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox22 wontfix, firefox23 wontfix, firefox24 wontfix, firefox25 fixed, firefox26 fixed, firefox-esr17 unaffected, firefox-esr2426+ verified)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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
(Reporter)

Updated

5 years ago
Keywords: hang
(Reporter)

Updated

5 years ago
Duplicate of this bug: 909074
(Reporter)

Comment 2

5 years ago
Created attachment 795140 [details]
testcase html
(Reporter)

Updated

5 years ago
Keywords: testcase
(Reporter)

Comment 3

5 years ago
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
(Assignee)

Comment 4

5 years ago
Created attachment 795163 [details] [diff] [review]
fix+tests

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)
(Assignee)

Comment 5

5 years ago
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)
(Assignee)

Comment 6

5 years ago
Created attachment 795235 [details] [diff] [review]
fix+tests, v2

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+
(Assignee)

Comment 8

5 years ago
(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.
(Assignee)

Comment 9

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
(Reporter)

Comment 11

5 years ago
Is this possible to uplift the patch to fix in Firefox24 and 24esr?
Flags: needinfo?(matspal)
(Assignee)

Comment 12

5 years ago
I think the patch would apply cleanly, but I don't think it meets the
branch criteria for 24.  Aurora perhaps...
Flags: needinfo?(matspal)
(Assignee)

Comment 13

5 years ago
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.
tracking-firefox24: --- → ?

Updated

5 years ago
Attachment #795235 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 15

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/889ee00342d6
status-firefox25: affected → fixed
status-firefox26: affected → fixed
status-firefox22: affected → wontfix
status-firefox23: affected → wontfix
status-firefox24: affected → wontfix

Updated

5 years ago
status-firefox-esr24: --- → affected
tracking-firefox24: ? → ---
tracking-firefox-esr24: --- → 25+
(Assignee)

Comment 16

4 years ago
Comment on attachment 795235 [details] [diff] [review]
fix+tests, v2

[Approval Request Comment]
see comment 13
Attachment #795235 - Flags: approval-mozilla-esr24?

Updated

4 years ago
tracking-firefox-esr24: 25+ → 26+

Updated

4 years ago
Attachment #795235 - Flags: approval-mozilla-esr24? → approval-mozilla-esr24+
(Assignee)

Comment 17

4 years ago
https://hg.mozilla.org/releases/mozilla-esr24/rev/20c584f64fec
status-firefox-esr24: affected → fixed
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.
status-firefox-esr24: fixed → verified
QA Contact: manuela.muntean
You need to log in before you can comment on or make changes to this bug.