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

cgi: Fix checkbox support (fixes #1008) #1010

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

zdohnal
Copy link
Member

@zdohnal zdohnal commented Jul 16, 2024

There was a change in CGI script regarding checkboxes, however it did not propagate into templates. Based on the change, the only valid check was if the variable value was checkbox, but some browsers (at least Firefox) send on as a default value for input form of type checkbox.

Additionally, the value checkbox looks like typo, because we use checked as value for checkboxes in admin CGI program, so I updated cgiGetCheckbox() as well.

To fix the behavior, we have to set VALUE="CHECKED" into every tag in every templates for all checkboxes - this value will be sent in the input form, so it will properly match with cgiGetCheckbox() logic now.

In the end, I have found out "Preserve Job History" checkbox from template was handled as text field, which did not look correct, and implemented default values for dependent options.

Fixes #1008

Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

See comments - main issue is the template changes - VALUE isn't valid for checkboxes, but the HIDDEN changes are OK.

cgi-bin/admin.c Outdated Show resolved Hide resolved
templates/add-printer.tmpl Show resolved Hide resolved
templates/admin.tmpl Show resolved Hide resolved
templates/admin.tmpl Show resolved Hide resolved
templates/admin.tmpl Show resolved Hide resolved
templates/admin.tmpl Show resolved Hide resolved
templates/da/add-printer.tmpl Show resolved Hide resolved
templates/da/admin.tmpl Show resolved Hide resolved
templates/da/admin.tmpl Show resolved Hide resolved
templates/da/admin.tmpl Show resolved Hide resolved
There was a change in CGI script regarding checkboxes, however it did
not propagate into templates. Based on the change, the only valid check
was if the variable value was checkbox, but some browsers (at least
Firefox) send on as a default value for input form of type checkbox.

Additionally, the value checkbox looks like typo, because we use checked
as value for checkboxes in admin CGI program, so I updated
cgiGetCheckbox() as well.

To fix the behavior, we have to set VALUE="CHECKED" into every tag in
every templates for all checkboxes - this value will be sent in the
input form, so it will properly match with cgiGetCheckbox() logic now.

In the end, I have found out "Preserve Job History" checkbox from
template was handled as text field, which did not look correct.

Fixes OpenPrinting#1008
Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

ok

@zdohnal zdohnal merged commit eabdc33 into OpenPrinting:master Jul 23, 2024
2 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to enable Sharing when modifying a printer from the CUPS web UI
2 participants