Opened 8 years ago

Last modified 6 weeks ago

#8349 confirmed defect

wxMac: correct min/max-values for printdialog

Reported by: sturmlechner Owned by:
Priority: normal Milestone:
Component: wxOSX (any toolkit) Version:
Keywords: Cc: sturmlechner, csomor, auria.mg@…
Blocked By: Blocking:
Patch: yes

Description

With wxMac the print dialog always shows a minimum value of 1 and a maximum value of 9999 to the user regardless of the true number of pages (this can be seen in the docview sample). With this patch the number of maximum pages is retrieved before the print dialog is opened.

Attachments (6)

printmac.patch download (3.0 KB) - added by sturmlechner 8 years ago.
wxprint.patch download (1.5 KB) - added by Auria 6 years ago.
Attempt at patch
wxOSXPrint.patch download (1.7 KB) - added by Auria 5 years ago.
improve code a bit (doesn't work unfortunately, though I don't think any fix can work without that one applied first)
wxOSXPrint2.patch download (842 bytes) - added by Auria 5 years ago.
Second part. ALong with the first part of the patch, posted earlier, fixes the problem. Needs review as I'm not sure if I broke something
wxOSX_print.patch download (2.2 KB) - added by Auria 4 years ago.
An updated version that applies cleanly on newer revisions of trunk
wxcocoa_printdlg_pages.patch download (828 bytes) - added by Auria 3 years ago.
A patch to complement my other ones so that the dialog no more shows 1/1 on Cocoa

Download all attachments as: .zip

Change History (30)

Changed 8 years ago by sturmlechner

comment:1 Changed 8 years ago by vadz

Could you please explain what exactly does this patch do, in particular why do we need to create the dialog twice?

Thanks!

comment:2 Changed 7 years ago by vadz

Unfortunately I can't apply this patch without any explanations...

comment:3 Changed 7 years ago by sf-robot

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

comment:4 Changed 6 years ago by Auria

  • Cc auria.mg@… added
  • Status changed from closed to reopened
  • Type set to defect

I believe this issue should be reopened. It is possible that the submitted patch is not applicable as-is, however the issue that is described in the first post still holds as of 2.8.9 and recent SVN (trunk, r57516). It is an important annoyance that my users are told 9999 pages are going to be printed

Bug report 9137 seems similar, however it is marked as fixed, and the issue described above is clearly not fixed since I can reproduce it.

Changed 6 years ago by Auria

Attempt at patch

comment:5 Changed 6 years ago by Auria

I attached a test patch. The problem was simply that the dialog was shown *before* the printout object was actually asked for page amount. I simply moved the GetPageInfo query to a location prior to showing the dialog, and it seems to work fine.

comment:6 Changed 6 years ago by vadz

  • Cc vadz removed
  • Patch unset

comment:7 Changed 5 years ago by Auria

  • Patch set

I'm not sure why "patch" was unset; I haven't found docs explaining what it is, but my best guess is that it indicates the post contains a patch. Since I posted a patch, I'm setting 'patch' on again.

comment:8 Changed 5 years ago by vadz

Sorry, I probably did it accidentally when removing myself from cc. I'll look at the patch now.

comment:9 Changed 5 years ago by VZ

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [62047]) Display correct minimal and maximal pages numbers under OS X.

Set min and max page fields in m_printDialogData before showing the print
dialog in the Mac version.

Closes #8349.

comment:10 Changed 5 years ago by Auria

  • Resolution fixed deleted
  • Status changed from closed to reopened

Unfortunately, this bug has reappeared in trunk.
This is because of http://trac.wxwidgets.org/ticket/11494 , where my change was reverted because my patch accidentally introduced a bug.
However, this bug still holds and I believe it still needs to be fixed (in a way that doesn't re-introduce bug #11494, of course).

I attached a patch that uses a better order without re-introducing #11494 . Unfortunately, the dialog still shows "from 1 to 1" even after my patch, as documented in #988. More investigation will be necessary to find why wxMacPrintDialog ignores the min and max values from 'm_printDialogData'

Changed 5 years ago by Auria

improve code a bit (doesn't work unfortunately, though I don't think any fix can work without that one applied first)

comment:11 Changed 5 years ago by Auria

I'm attaching a second patch that, added to the first, brings the correct min/max values back.
Now, I just commented out something. I am not sure why this code was written this way in the first place, so my patch should be reviewed first by someone who knows this code, in case I broke something.

Changed 5 years ago by Auria

Second part. ALong with the first part of the patch, posted earlier, fixes the problem. Needs review as I'm not sure if I broke something

Changed 4 years ago by Auria

An updated version that applies cleanly on newer revisions of trunk

comment:12 Changed 3 years ago by vadz

  • Status changed from reopened to infoneeded_new

I don't see the problem neither in docview nor in the printing sample with the current trunk. Admittedly, their behaviour is still not what I'd expect as when I expand the "details" area of the native print dialog I see the range of pages as being "1 to 1" and not "1 to 2" in the printing sample as expected. But at least I don't see 9999 anywhere...

And the observed behaviour under OS X 10.6 doesn't change with or without the wxOSX_print.patch applied.

Am I missing something?

comment:13 Changed 3 years ago by Auria

  • Owner csomor deleted
  • Status changed from infoneeded_new to new

Indeed with latest SVN the 9999 is gone, which is a good thing; however that's only because the print dialog is now broken and shows "1" no matter what value you pass :(

I tried adding some logging in printmac.cpp just before the dialog is shown :

    printf("Min page : %i\n", m_printDialogData.GetMinPage());
    printf("Max page : %i\n", m_printDialogData.GetMaxPage());

Without my patch, it says :

Min page : 1
Max page : 9999

with my patch it says :

Min page : 1
Max page : 2

So really it's not that latest SVN is any better than before, the 9999 is still there : it's actually that in latest SVN there is yet another bug that hides the 9999 bug :(

In any case, my patch brings the issue closer to being fixed since it at least ensures m_printDialogData contains the right number - one my patch is applied then we need to make the dialog actually read the numbers from m_printDialogData instead of showing 1/1

Changed 3 years ago by Auria

A patch to complement my other ones so that the dialog no more shows 1/1 on Cocoa

comment:14 Changed 3 years ago by Auria

I have added a new patch that corrects the min/max page display in the Cocoa print dialog. (Unfortunately this patch is not ideal because design-wise this code should go in wxPrintData::ConvertToNative() - it's just that ConvertToNative is written using Carbon and I have neraly zero Carbon knowledge. But at least it should give a good idea how to proceed to fix the numbers in the dialog)

comment:15 Changed 3 years ago by Auria

On second thought, I was wrong, this code cannot go in wxPrintData::ConvertToNative(), simply because the min page and max page fields are not found in wxPrintData. So to the extent of my limited wxWidgets internals knowledge, I would deem my patches as appropriate

comment:16 Changed 3 years ago by Auria

  • Component changed from wxOSX-Carbon to wxOSX (any toolkit)

comment:17 Changed 2 years ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [72805]) Fix the pages range in the print dialog in wxOSX.

Set the min/max pages earlier for them to be taken into account and also
actually do set them in the print settings.

Closes #8349.

comment:18 Changed 2 years ago by vadz

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'll have to revert this because it results in crashes when using wxHtmlEasyPrinting (and probably not only it), see #11779.

comment:19 Changed 2 years ago by VZ

(In [72883]) Revert "Fix the pages range in the print dialog in wxOSX."

This reverts r72805 (leaving only the changes to printdlg.cpp which seem
harmless and potentially useful) as it resulted in crashes when using
wxHtmlEasyPrinting because we called wxPrintout::OnPreparePrinting() before
setting the DC to be used, which was wrong.

In fact it's not clear how can we get the correct range of pages at all
because we need a DC to paginate properly (i.e. taking into account its size)
but we need to show a dialog, in which we already want to show the pages
range, before choosing the DC. Perhaps we could create a dummy DC for
pagination purposes but how could this work with printers using different page
sizes?

The best would probably be to avoid setting any limits on the page range as
showing 9999 looks ugly but anything else would be wrong.

See #8349, #11779.

comment:20 Changed 2 years ago by Auria

This sounds like the same chicken-and-egg problem described in #14136

But I wonder, maybe I'm wrong, but at the time this dialog is shown, I believe the "page setup" information is known. So maybe a correct DC could be created?

comment:21 Changed 2 years ago by vadz

  • Status changed from reopened to confirmed

I'm not sure how could this work, isn't the page size dependent on the printer that will be (but hasn't been yet) chosen?

Anyhow, if you can find some way to make this work, it would be nice because seeing 9999 in the dialog is definitely bad, but please check that it still works with wxHtmlEasyPrinting.

comment:22 Changed 23 months ago by Auria

Well as far as I know, wxWidgets still uses the "old-style" printing, where "page setup" is called first, and then the printing dialog. In this way, IIUC, page size is selected first in the "page setup" dialog. Only then is the print dialog shown.

Of course there is the quirk case where you select a paper size, then in the following dialog select a printer that can't actually print that size. I'm not sure this is handled correctly at all to be honest. In the optimal situation, wxWidgets would be updated to work like the more "modern" printing dialogs with a single dialog, and the pagecount/preview updating dynamically as you tweak settings. But this is probably way too much work to expect that for 3.0.

So I guess we should check what happens if in page setup you select a paper size, then in print select a printer that can't actually print that size. Probably this should be checked for each OS. If it turns out wx only supports selecting printers that can actually print the selected paper size, I see no issue

comment:23 Changed 6 weeks ago by oneeyeman

Running printing sample of Cocoa build and selecting "File->Print Preview..." correctly shows that the number of pages is 2.
Moreover under Cocoa, the print dialog does not let you select the number of pages, as running printing sample and selecting "File->Print..." only let you change printer and presets. It also lets you print to PDF format or go to the preview, which correctly shows number of pages as 2.

However, I did not check wxHTMLEasyPrinting class.

So unless I'm missing something, this bug should be closed.

comment:24 Changed 6 weeks ago by oneeyeman

Running htmlprinting sample under Cocoa build and selecting "File->Print Preview" I se number of pages reported as 5.
So there is no bug there as well.

Note: See TracTickets for help on using tickets.