Closed
Bug 1328275
Opened 4 years ago
Closed 4 years ago
Refactor and comment nsSimplePageSequenceFrame::PrintNextPage to make it less difficult to understand
Categories
(Core :: Printing: Output, defect)
Core
Printing: Output
Tracking
()
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: jwatt, Assigned: jwatt)
Details
Attachments
(1 file, 1 obsolete file)
7.87 KB,
patch
|
bobowen
:
review+
|
Details | Diff | Splinter Review |
Spinning this out from bug 1326418 comment 6.
Assignee | ||
Comment 1•4 years ago
|
||
Attachment #8823258 -
Flags: review?(bobowencode)
Comment 2•4 years ago
|
||
Comment on attachment 8823258 [details] [diff] [review] patch Review of attachment 8823258 [details] [diff] [review]: ----------------------------------------------------------------- This looks like it implements the same behaviour, apart from not leaving the loop if we have more that one page. ::: layout/generic/nsSimplePageSequenceFrame.cpp @@ +718,3 @@ > nsDeviceContext* dc = PresContext()->DeviceContext(); > > + nsPageFrame * pf = static_cast<nsPageFrame*>(currentPageFrame); nit: space before pointer @@ +776,5 @@ > nsDisplayListBuilderMode::PAINTING, > nsLayoutUtils::PaintFrameFlags::PAINT_SYNC_DECODE_IMAGES); > > if (mSelectionHeight >= 0 && selectionY < mSelectionHeight) { > + haveUnfinishedSelectionToPrint = true; I can't see where this ever gets set back to false to exit the loop.
Attachment #8823258 -
Flags: review?(bobowencode) → review-
Assignee | ||
Comment 3•4 years ago
|
||
Yikes!
Attachment #8823258 -
Attachment is obsolete: true
Attachment #8823302 -
Flags: review?(bobowencode)
Comment 4•4 years ago
|
||
Comment on attachment 8823302 [details] [diff] [review] patch Review of attachment 8823302 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/nsSimplePageSequenceFrame.cpp @@ +777,5 @@ > nsDisplayListBuilderMode::PAINTING, > nsLayoutUtils::PaintFrameFlags::PAINT_SYNC_DECODE_IMAGES); > > + if (mSelectionHeight >= 0) { > + haveUnfinishedSelectionToPrint = (selectionY < mSelectionHeight); This still feels slightly wrong, but maybe it doesn't matter in reality. At the moment mSelectionHeight is only set up front as far as I can find. But if that changed for some reason and it was set back to its default of -1 while we were in here, then we could still loop.
Attachment #8823302 -
Flags: review?(bobowencode) → review+
Pushed by jwatt@jwatt.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/f43e221412b3 Refactor and comment nsSimplePageSequenceFrame::PrintNextPage to make it easier to understand. r=bobowen
Comment 6•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f43e221412b3
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•