Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dev/core#2959: some PDFs hard-code format to "a3 landscape", overriding default PDF format #22098

Merged

Conversation

twomice
Copy link
Contributor

@twomice twomice commented Nov 18, 2021

Overview

See the original issue at https://lab.civicrm.org/dev/core/-/issues/2959. In short: civicrm normally specifies default pdf format according to whatever is configured in the UI (if any). But in some cases it is hard-coded to use "A3 Landscape" -- e.g. when calling an existing CiviCRM URL with "?snippet=3" in order to generate a pdf. (We're doing this on some sites in order to print Event Participant Listings as PDF file.)

This happens because there's actually code in CiviCRM that says "use A3 Landscape", and this code is invoked in workflows like described above.

Before

  1. Append "?snippet=3" to any civicrm url and observe that you're prompted to save a pdf file.
  2. Open that pdf file and observe it has page size A3 and orientation Landscape. This is regardless of whatever default PDF format may be configured at /civicrm/admin/pdfFormats?reset=1

After

  1. Append "?snippet=3" to any civicrm url and observe that you're prompted to save a pdf file.
  2. Open that pdf file and observe it has page size, orientation, and other properties defined in the default PDF format configured at /civicrm/admin/pdfFormats?reset=1 (and if none is configured, then whatever defaults are hard-coded in CRM_Core_BAO_PdfFormat::$optionValueFields).

Tech details

Hard to guess why this code was written in the first place (esp. since it predates SVN-to-git migration, see it in 6a488035). It seems fairly uncommon to keep A3 paper in the printer by default.

Comments

I've at least got a "sounds good" from @jaapjansma in the issue comments: https://lab.civicrm.org/dev/core/-/issues/2959#note_67495

@civibot
Copy link

civibot bot commented Nov 18, 2021

(Standard links)

@civibot civibot bot added the master label Nov 18, 2021
@eileenmcnaughton
Copy link
Contributor

I agree with your analysis so with @jaapjansma that makes 3 of us - I think that's enough consensus for something this obscure

@eileenmcnaughton eileenmcnaughton merged commit 456cc1d into civicrm:master Nov 18, 2021
@twomice
Copy link
Contributor Author

twomice commented Nov 18, 2021

Thanks @eileenmcnaughton !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants