Closed Bug 909078 Opened 8 years ago Closed 8 years ago
Browser hangs attempting to print preview on certain page
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
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
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...
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.
OS: Windows 7 → All
Hardware: x86_64 → All
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?
I think the patch would apply cleanly, but I don't think it meets the branch criteria for 24. Aurora perhaps...
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+
You need to log in before you can comment on or make changes to this bug.