Closed Bug 1341166 Opened 7 years ago Closed 1 year ago

Handle printing all pages of PDF file on Mac

Categories

(Core :: Widget: Cocoa, defect, P3)

45 Branch
Unspecified
macOS
defect

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: lochang, Unassigned)

Details

      No description provided.
No longer depends on: 1331321
Value of |printRange| of print settings is wrong on Mac. It should be 0 (kRangeAllPages) when printing all pages. But it always return 1 (kRangeSpecifiedPageRange).
Hi Markus,

I have a question about printing on Mac, could you help me out? Thanks.

I am now working on printing of PDF. And i try to call the existing XPCOM |showPrintDialog| from our javascript runtime to get user's print settings. And the |printRange| of print settings I got is always 1.
And I look into the code, I find that the |lastPage| got from dialog is INT_MAX but not UINT32_MAX [1]. So maybe we should fix it as below:

status = ::PMGetLastPage(nativePrintSettings, &lastPage);
if (status == noErr) {
  aSettings->SetStartPageRange(firstPage);
  aSettings->SetEndPageRange(lastPage);
  if (lastPage != INT_MAX) {
    aSettings->SetPrintRange(nsIPrintSettings::kRangeSpecifiedPageRange);
  } else {
    aSettings->SetPrintRange(nsIPrintSettings::kRangeAllPages);
  }
}
Does that make sense to you?

[1] http://searchfox.org/mozilla-central/source/widget/cocoa/nsPrintDialogX.mm#169-174
Flags: needinfo?(mstange)
Assignee: nobody → lochang
Status: NEW → ASSIGNED
It returns INT_MAX? That's not very helpful... Does it return that value even if you specify a different value in the textfield? This doesn't really match the documentation at https://developer.apple.com/reference/applicationservices/1462747-pmgetlastpage?language=objc ...
Flags: needinfo?(mstange)
(In reply to Markus Stange [:mstange] from comment #3)
> It returns INT_MAX? That's not very helpful... Does it return that value
> even if you specify a different value in the textfield? This doesn't really
> match the documentation at
> https://developer.apple.com/reference/applicationservices/1462747-
> pmgetlastpage?language=objc ...

Yep... it returns INT_MAX. I was confused at the beginning as well. So the value of lastPage will be INT_MAX if we are printing 'all' pages, and it would be correct if we choose a specific print range. Besides, I think the document only mention that the type of lastPage should be UINT32, but in the discussion, it says if we doesn't specify a last page through PMSetLastPage it will return a default value which document doesn't mention what the default value is. Or maybe we can set a default value to last page before we show the dialog?
However, here is another open source example that use INT_MAX to determine if user is printing 'all' pages [1].

As for printing on Mac work fine now, it checks if the lastPage is larger than the actual pageNum. If does, it will set the lastPage to the pageNum. So printing works fine no matter how big the lastPage is (INT_MAX or UINT32_MAX) [2].

[1] https://github.com/wxWidgets/wxWidgets/blob/master/src/osx/core/printmac.cpp#L490
[2] http://searchfox.org/mozilla-central/source/layout/printing/nsPrintEngine.cpp#2706-2708
Flags: needinfo?(mstange)
Sorry for my long silence here.

So can you do what we are already doing? Compare lastPage to the number of pages, and set the range to "all pages" if lastPage is larger? Then we don't rely on the magic number.
Flags: needinfo?(mstange)
(In reply to Markus Stange [:mstange] from comment #5)
> So can you do what we are already doing? Compare lastPage to the number of
> pages, and set the range to "all pages" if lastPage is larger? Then we don't
> rely on the magic number.

I guess we are not able to get the number of pages here in nsPrintDialogServiceX::Show, since OS doesn't know the number of pages. Or you mean we do the checking similar to nsPrintEngine::PrintPage ourselves when printing, and nsPrintDialogServiceX::Show remains the same?
Flags: needinfo?(mstange)
To make it clear. The reason why current printing behavior on Mac is correct is because we always enter the isDoingPrintRange 'if' block [1] no matter printing all pages or specified pages. And luckily we check if lastpage (INT32_MAX here) is larger than number of pages. If does we set lastpage to number of pages.

But I think the correct flow should be if we are printing all pages we enter the 'else' block [2] and directly set the lastpage to number of pages since the value of lastpage is meaningless INT32_MAX. So we have to fix the printRange in nsPrintDialogX.mm as I suggested in comment 2.

[1] http://searchfox.org/mozilla-central/source/layout/printing/nsPrintEngine.cpp#2704-2720
[2] http://searchfox.org/mozilla-central/source/layout/printing/nsPrintEngine.cpp#2721-2727
Priority: -- → P2
Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Assignee: lochang → nobody
Status: ASSIGNED → NEW
Severity: normal → S3
Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(mstange.moz)
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.