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

Printer status special character support #689

Merged
merged 42 commits into from
Jul 18, 2020
Merged

Printer status special character support #689

merged 42 commits into from
Jul 18, 2020

Conversation

Vzor-
Copy link
Contributor

@Vzor- Vzor- commented Jul 14, 2020

fixes #668

Ready for merge.

Fixes:

  • Mac os printer names reported by status listeners now agree with the names given by 'find all printers'.
  • Fixed an issue where '&' was breaking the status reporting on Ubuntu (and probably mac)
  • Some erroneous html escaping in sample.html

Known Issues:

  • \ ` ^ + and probably some other symbols break the jfx PrintServiceLookup, this issue also affects "find all printers"
  • Changing printer names on mac doesn't affect the names returned by status until qz restarts. This is due to there being no warning that a printer's description was changed, and renaming a printer on mac only changes the description.

Todo:

  • Investigate solution to mac renaming issue
  • Test changes on windows

@@ -54,7 +59,7 @@ public static PrinterStatus getFromCupsString(String reason, String issuingPrint
}

public String toString() {
String returnString = type.getName() + ": Level " + type.getSeverity() + ", StatusCode " + type.getCode() + ", From " + issuingPrinterName;
String returnString = type.getName() + ": Level " + type.getSeverity() + ", StatusCode " + type.getCode() + ", From " + issuingPrinterDescription;
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid confusion please keep issuingPrinterName in the logs. If description is desired, add it as an additional field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason this changed is because this is the status' toString, and I wanted it to reflect exactly what the status will report to the client. Should I still go ahead with this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason this changed is because this is the status' toString, and I wanted it to reflect exactly what the status will report to the client. Should I still go ahead with this change?

It's just misleading that on Linux, it will use a field labeled "description", when it's clearly not.

@tresf tresf merged commit c4751f6 into qzind:master Jul 18, 2020
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.

Printer status should send human-friendly printer name
2 participants