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

Bare staple value not handled in finishings-supported #1073

Closed
yetamrra opened this issue Oct 4, 2024 · 5 comments · Fixed by #1094
Closed

Bare staple value not handled in finishings-supported #1073

yetamrra opened this issue Oct 4, 2024 · 5 comments · Fixed by #1094
Assignees
Labels
bug Something isn't working priority-high
Milestone

Comments

@yetamrra
Copy link
Contributor

yetamrra commented Oct 4, 2024

I have encountered a driverless printer that includes staple in the finishings-supported attribute, but doesn't have any of the other staple-XXX values. The PPD generated by cupsd doesn't contain any of the *StapleLocation or *cupsIPPFinishings keywords that normally get generated for other printers.

Taking a look at the section starting at cups/ppd-cache.c:4517, it seems like this is supposed to work by mapping through the SingleAuto keyword. But the two checks on lines 4553 and 4600 only match values that start with staple-, so the bare staple value never gets handled. Locally patching in a check for strcmp("staple") makes things behave the way I would expect and also looks similar to the checks done for other finishings later in the same file, but I'm not sure if I might be missing something here.

Is the bare staple value supposed to work when the other staple-XXX values aren't present? I couldn't find anything in the IPP Finishings doc saying it shouldn't, but finishings has been covered in enough different places that I don't have confidence I read everything relevant. If it is supposed to work, do you want me to send a quick PR to add the missing value to the checks?

The same issue might apply to a bare bind value based on the similar pattern in the same checks.

I've attached an ippserver config captured from this printer in case you want to look through the whole set of attributes.
HL-L6415DW.conf.txt

@michaelrsweet michaelrsweet self-assigned this Oct 4, 2024
@michaelrsweet michaelrsweet added bug Something isn't working priority-high labels Oct 4, 2024
@michaelrsweet michaelrsweet added this to the v2.4.x milestone Oct 4, 2024
@michaelrsweet
Copy link
Member

Confirmed, will also make sure the base "punch" and "fold" values are covered as well.

@yetamrra
Copy link
Contributor Author

Just wanted to double-check on this. Would it be helpful for me to send a PR?

@zdohnal
Copy link
Member

zdohnal commented Oct 31, 2024

@yetamrra PRs are always welcome!

yetamrra added a commit to yetamrra/cups that referenced this issue Nov 1, 2024
In the generated PPD, IPP finishings "bind" is supposed to map to
"StapleLocation: BindAuto".  Similarly, "staple" is supposed to map to
"StapleLocation: SingleAuto".  The code already handles this, except
the lookup is blocked by a check that only accepts "staple-*" and
"bind-*" prefixed versions.  Fix this by adding the bare versions to the
existing checks.

Fixes issue OpenPrinting#1073.
@yetamrra
Copy link
Contributor Author

yetamrra commented Nov 1, 2024

Sure thing. I sent #1094 with a proposed fix.

@michaelrsweet michaelrsweet linked a pull request Nov 15, 2024 that will close this issue
@yetamrra
Copy link
Contributor Author

Sorry, I forgot to post a follow-up here. We've cherry picked the fix into the 2.4 branch on ChromeOS and confirmed that it works as expected with the original printer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants