Closed Bug 127145 Opened 23 years ago Closed 22 years ago

print problems when paginating a div that does not fit on a page (it does not split correctly)

Categories

(Core :: Layout, defect, P2)

x86
Windows 2000
defect

Tracking

()

RESOLVED FIXED
mozilla1.2beta

People

(Reporter: alexsavulov, Assigned: karnaze)

References

Details

(Keywords: topembed+, Whiteboard: PATCH)

Attachments

(4 files, 4 obsolete files)

problems paginating a div that does not fit on a page (it does not split correctly) testcases to follow
Attached file testcase with less content than hight (deleted) —
Summary: problems paginating a div that does not fit on a page (it does not split correctly) → print problems when paginating a div that does not fit on a page (it does not split correctly)
Priority: -- → P3
Changing QA contact
QA Contact: petersen → amar
*** Bug 139217 has been marked as a duplicate of this bug. ***
nsbeta1+.
Assignee: alexsavulov → karnaze
Keywords: nsbeta1+
Keywords: topembed
Severity: normal → major
Status: NEW → ASSIGNED
Priority: P3 → P2
Target Milestone: --- → mozilla1.2beta
Attached image another test case (obsolete) (deleted) —
Attached file another test case (deleted) —
Attachment #102539 - Attachment is obsolete: true
Attached patch patch to fix the bug (obsolete) (deleted) — Splinter Review
Attached patch revised patch (obsolete) (deleted) — Splinter Review
Attachment #102542 - Attachment is obsolete: true
Whiteboard: PATCH
Keywords: topembedtopembed+
Comment on attachment 102569 [details] [diff] [review] revised patch r= alexsavulov
Attachment #102569 - Flags: review+
Attached patch revised patch with kin's suggestions (obsolete) (deleted) — Splinter Review
Attachment #102569 - Attachment is obsolete: true
Attachment #102874 - Flags: review+
Comment on attachment 102874 [details] [diff] [review] revised patch with kin's suggestions ==== It was bugging me that the |aMetrics.height| for the last-in-flow still contained the |borderPadding.top| after all the prev-in-flow heights were subtracted off, especially since in my mind all next-in-flows for the block frame should be at the top of the page with no top border drawn, so there shouldn't be a need for the |borderPadding.top| to be applied? So I did some experiments and found out that right now the |borderPadding.top| is actually applied to all next-in-flows for the block. This means that in the case where the computed height is constrained, that we will end up with a height for the last-in-flow which is too small by roughly |numPrevInFlows*borderPadding.top|. I'll attach a test case to illustrate what I'm talking about. I really think that |borderPadding.top| should only be applied to the primary frame, since it should be the only one that gets the top of the border, but I'm not exactly sure if you want to try and tackle that in the context of this bug. In any case, I think this warrants some sort of XXX comment. Here's the little change I made to get my test case working: + for (nsIFrame* prev = mPrevInFlow; prev; prev->GetPrevInFlow(&prev)) { + nsRect rect; + prev->GetRect(rect); + aMetrics.height -= rect.height; + + // XXX: Right now all block next-in-flows have the borderPadding.top + // applied to them, we should remove the following when this gets + // fixed. + aMetrics.height += borderPadding.top; + } You'll notice that when I mentioned |numPrevInFlows*borderPadding.top|, I said it was *roughly* equivalent, since using print preview, I noticed there was a slight increase in the space between the last line of text and the bottom border. Comments? ==== The setting of |aStatus| should probably be before the |#ifdef DEBUG| block which prints it's value out. @@ -1131,6 +1129,7 @@ } #endif + aStatus = state.mReflowStatus; NS_FRAME_SET_TRUNCATION(aStatus, aReflowState, aMetrics); return rv; }
Attached file Test case with top padding (deleted) —
Here's the test case I mentioned above.
Comment on attachment 102964 [details] Test case with top padding doh, I just realized I submitted the test case with an extremely large border width I used when poking around. It doesn't need to be that big to see the problem, it can even be 1px.
Yes, there is a bug in the block/inline code where the top border padding space gets used in layout even for next-in-flows which don't have a top border or padding. I filed bug 174688 to deal with the problem. I even tried fixing the problem here, but there are too many places where this happens and I gave up, not wanting to increase the risk of this patch for m1.2.
Attachment #102874 - Attachment is obsolete: true
Comment on attachment 103028 [details] [diff] [review] revised patch with kin's latest suggestions r= alexsavulov
Attachment #103028 - Flags: review+
Comment on attachment 103028 [details] [diff] [review] revised patch with kin's latest suggestions sr=kin@netscape.com
Attachment #103028 - Flags: superreview+
I have tested the build from Karnaze with the patch for possible regressons. I dint find any regressions..we can check the patch into the trunk and wait for a while before we check into the branch builds
Comment on attachment 103028 [details] [diff] [review] revised patch with kin's latest suggestions a=asa for checkin to 1.2 (on behalf of drivers)
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Blocks: grouper
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: