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)
Tracking
()
RESOLVED
FIXED
mozilla1.2beta
People
(Reporter: alexsavulov, Assigned: karnaze)
References
Details
(Keywords: topembed+, Whiteboard: PATCH)
Attachments
(4 files, 4 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
alexsavulov
:
review+
kinmoz
:
superreview+
|
Details | Diff | Splinter Review |
problems paginating a div that does not fit on a page (it does not split correctly)
testcases to follow
Reporter | ||
Comment 1•23 years ago
|
||
Reporter | ||
Updated•23 years ago
|
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)
Updated•23 years ago
|
Priority: -- → P3
Comment 3•23 years ago
|
||
*** Bug 139217 has been marked as a duplicate of this bug. ***
Comment 5•22 years ago
|
||
batch: adding topembed per Gecko2 document
http://rocknroll.mcom.com/users/marek/publish/Gecko/Gecko2Tasks.html
Keywords: topembed
Assignee | ||
Updated•22 years ago
|
Severity: normal → major
Status: NEW → ASSIGNED
Priority: P3 → P2
Target Milestone: --- → mozilla1.2beta
Assignee | ||
Comment 6•22 years ago
|
||
Assignee | ||
Comment 7•22 years ago
|
||
Attachment #102539 -
Attachment is obsolete: true
Assignee | ||
Comment 8•22 years ago
|
||
Assignee | ||
Comment 9•22 years ago
|
||
Attachment #102542 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Whiteboard: PATCH
Updated•22 years ago
|
Reporter | ||
Comment 10•22 years ago
|
||
Comment on attachment 102569 [details] [diff] [review]
revised patch
r= alexsavulov
Attachment #102569 -
Flags: review+
Assignee | ||
Comment 11•22 years ago
|
||
Attachment #102569 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #102874 -
Flags: review+
Comment 12•22 years ago
|
||
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;
}
Comment 13•22 years ago
|
||
Here's the test case I mentioned above.
Comment 14•22 years ago
|
||
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.
Assignee | ||
Comment 15•22 years ago
|
||
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.
Assignee | ||
Updated•22 years ago
|
Attachment #102874 -
Attachment is obsolete: true
Reporter | ||
Comment 16•22 years ago
|
||
Comment on attachment 103028 [details] [diff] [review]
revised patch with kin's latest suggestions
r= alexsavulov
Attachment #103028 -
Flags: review+
Comment 17•22 years ago
|
||
Comment on attachment 103028 [details] [diff] [review]
revised patch with kin's latest suggestions
sr=kin@netscape.com
Attachment #103028 -
Flags: superreview+
Comment 18•22 years ago
|
||
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 19•22 years ago
|
||
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)
Assignee | ||
Comment 20•22 years ago
|
||
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•