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 Printing Manifest UserPrinterList #585

Merged
merged 1 commit into from
Dec 4, 2022
Merged

Fix Printing Manifest UserPrinterList #585

merged 1 commit into from
Dec 4, 2022

Conversation

apizz
Copy link
Collaborator

@apizz apizz commented Dec 4, 2022

Resolves #584

Was not correctly setup to be an array of dictionaries. Changes correctly show this in the UI.

Screenshot 2022-12-03 at 11 25 00 PM

Also:

  • Add a pfm_exclude for DefaultPrinter since this needs to be in the UserPrinterList
  • Update some descriptions for clarity

- Fix to be array of dictionaries & add missing `pfm_hidden` of `container`
- Add a `pfm_exclude` for `DefaultPrinter` since this needs to be in the UserPrinterList
- Update some descriptions
@apizz apizz added the 🛠️ update manifest Update existing manifest label Dec 4, 2022
@apizz apizz self-assigned this Dec 4, 2022
Copy link
Member

@kevinmcox kevinmcox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the fix @apizz.

@apizz apizz merged commit c7b8d10 into master Dec 4, 2022
@relgit
Copy link
Collaborator

relgit commented Dec 6, 2022

@apizz, @kevinmcox, I think this needs another look. The key is documented as a dictionary and changing it to array is incorrect (@DigiYann ran tests yesterday to confirm).

The previous manifest allowed entering one printer, and in the profile this was represented by a dictionary where the configured printer appeared under the key Printer. In reality this key was more akin to an identifier than to a preference key because it could be anything, and Printer was just an example provided by the manifest. Adding more printers was possible in a text editor provided that each also had its own key.

The conventional way to represent this in the manifest is by changing the example key name from Printer to {{key}}, but that would do us no good at all because such a structure is so rare that neither iMazing Profile Editor nor ProfileCreator support it right now.

I've considered suggesting that we relax the pfm_unique restriction so that users could add multiple printers by adding multiple payloads but testing showed this approach to fail as well.

Perhaps you guys have other ideas to try, but otherwise I recommend that we revert this PR and keep the issue open.

@apizz
Copy link
Collaborator Author

apizz commented Dec 6, 2022

My apologies for the last comment. I wasn't clear on what you were saying, but I can see now that I made an error and that in fact the UserPrinterList dictionary contains not just other dictionaries which define printers but also a KEY for these. So the implementation "fixed" the visual issue, but subsequently has changed the format of the output payload such that this won't actually work.

Will need to make changes here to get this working again.

@everetteallen
Copy link

@apizz is anyone working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ update manifest Update existing manifest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

com.apple.mcxprinting does not allow multiple printers...
4 participants