Fixes in "printers@cinnamon.org"#13524
Fixes in "printers@cinnamon.org"#13524hans-fritz-pommes wants to merge 20 commits intolinuxmint:masterfrom
Conversation
|
Oh and I changed icons. If you think they're ugly, just revert it |
|
I have an idea how to fix this mini-problem (identical bytes & job-number). But later |
Fix mini-problem (See linuxmint#13524)
|
Ok, now the identical bytes/document-name & job-number-problem is fixed. |
|
Sorry that I'm so chaotic |
Support filenames with whitespaces
files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js
Outdated
Show resolved
Hide resolved
files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js
Outdated
Show resolved
Hide resolved
files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js
Outdated
Show resolved
Hide resolved
Best-practices scannerThis is a regex-based check for API usage that can pose security, performance or This check is not perfect will not replace a normal review.Found 1 potential issue(s):
|
Improved with mtwebsters ideas (linuxmint#13524)
Best-practices scannerThis is a regex-based check for API usage that can pose security, performance or This check is not perfect will not replace a normal review.Found 1 potential issue(s):
|
What should be done with that? |
|
Please squash the commits into one. |
What it suggests - use an arrow function instead of using Lang.bind(). Look at other applets, it's a pretty straightforward change. |
removed deprecated lang.bind
|
By the way: |
Adressed linuxmint#9265
Adressing linuxmint#9265 Necessary update to catch the error reason
|
Those new commits intend to make it compatible with a turned-off CUPS-service |
🤦oups merging is not my task, right? |
improved error handling
improved error handling
added this.removed
removed unnecessary timeout
improved looking of error-handling
files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js
Outdated
Show resolved
Hide resolved
|
Can we drop the python script? |
files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js
Outdated
Show resolved
Hide resolved
- exchanged "Util.spawn_async(...)" & "tool.py" with "Util.spawnCommandLineAsyncIO(...)" - exchanged "GLib.format_size_for_display(...)" with "GLib.format_size(...)" - exchanged "cancel-print-dialog.py" with "ModalDialog.ConfirmDialog"
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Best-practices scannerThis is a regex-based check for API usage that can pose security, performance or This check is not perfect will not replace a normal review.Found 5 potential issue(s): ℹ️ shell_string_spawnfiles/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:181 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:192 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:214 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:242 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:265 Prefer argument vector spawn functions over shell command strings. Automated pattern check. |
Why would it be insecure if there were user inputs? (It's just about understanding) |
|
Ah and I replaced the second python script - this means now we don't have Gtk-dialogs anymore. Instead: A Cinnamon modal dialog. |
replaced error message cut 69 with 70 removed unnecessary line with printError = falsr;
This comment was marked as outdated.
This comment was marked as outdated.
|
Should I port to |
Best-practices scannerThis is a regex-based check for API usage that can pose security, performance or This check is not perfect will not replace a normal review.Found 5 potential issue(s): ℹ️ shell_string_spawnfiles/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:180 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:191 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:213 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:241 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:264 Prefer argument vector spawn functions over shell command strings. Automated pattern check. |
|
oups - not intentionally updated my fork |
You can leave as is for now, unless you're already running the latest Cinnamon git master yourself... it's an easy fix down the road. I'm a bit more concerned about the I had a graphic generated: flowchart TD
Start["update()"] --> Guard{"updating OR<br/>menu.isOpen OR<br/>removed?"}
Guard -->|Yes| Bail["return (skip)"]
Guard -->|No| Init["updating = true<br/>menu.removeAll()<br/>Add 'Printers' item<br/>out1=[] out2='' out3=[]"]
Init --> LpstatA["<b>lpstat -a</b><br/>(get available printers)"]
LpstatA -->|error| ErrA["handleError(stderr)<br/>⚠️ menu.removeAll()<br/>updating = false"]
LpstatA -->|success| StoreOut1["this.out1 = printer list"]
StoreOut1 --> Fork["<b>⚡ TWO ASYNC CALLS<br/>LAUNCHED IN PARALLEL</b>"]
Fork --> BranchA["<b>BRANCH A</b><br/>lpstat -l<br/>(tooltip/icon)"]
Fork --> BranchB["<b>BRANCH B</b><br/>lpstat -d<br/>(default printer)"]
%% Branch A
BranchA -->|error| ErrL["handleError(stderr)<br/>⚠️ menu.removeAll()<br/>⚠️ updating = false"]
BranchA -->|success| IconUpdate["Set tooltip<br/>Set icon based on<br/>⚠️ this.jobs.length<br/>(STALE - not yet
populated!)"]
%% Branch B - main chain
BranchB -->|error| ErrD["handleError(stderr)<br/>⚠️ menu.removeAll()<br/>⚠️ updating = false"]
BranchB -->|success| Printers["Add separator<br/>Add printer items to menu<br/>Store this.printers"]
Printers --> LpstatO["<b>lpstat -o</b><br/>(get job list)"]
LpstatO -->|error| ErrO["handleError(stderr)<br/>updating = false"]
LpstatO -->|success| Jobs["Store this.out3<br/>Add 'Cancel all' menu<br/>Add cancel submenu"]
Jobs --> LpqA["<b>lpq -a</b><br/>(get job details)"]
LpqA -->|error| LpqFail["lpq_present = false<br/>(graceful, no handleError)"]
LpqA -->|success| LpqOk["lpq_present = true<br/>Parse job info"]
LpqFail --> BuildJobs
LpqOk --> BuildJobs
BuildJobs["Build job menu items<br/>Populate this.jobs[]<br/>⚠️ jobInfo#91;job#93; may be undefined!"] -->
FinalUI["Set label (job count)<br/>Set visibility<br/>Open submenus<br/><b>updating = false</b>"]
%% Highlight race conditions
style Fork fill:#ff6b6b,color:#fff
style ErrL fill:#ff6b6b,color:#fff
style IconUpdate fill:#ffaa00,color:#000
style BuildJobs fill:#ffaa00,color:#000
style ErrA fill:#ff6b6b,color:#fff
style ErrD fill:#ff6b6b,color:#fff
style ErrO fill:#ff6b6b,color:#fff
The Problems IllustratedRace condition (red nodes): After lpstat -a succeeds, lpstat -l (Branch A) and lpstat -d (Branch B) are launched
The fix would be to make lpstat -l part of the sequential chain rather than parallel — e.g., call it at the very end after |
|
Yes, I was intending to make it faster - which is a very small advantage compared to the possible problems... |
changed parallel "lpstat -l"-call to a "linear" call
Best-practices scannerThis is a regex-based check for API usage that can pose security, performance or This check is not perfect will not replace a normal review.Found 5 potential issue(s): ℹ️ shell_string_spawnfiles/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:180 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:191 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:219 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:242 Prefer argument vector spawn functions over shell command strings. files/usr/share/cinnamon/applets/printers@cinnamon.org/applet.js:309 Prefer argument vector spawn functions over shell command strings. Automated pattern check. |
|
Sadly, I don't know how to test the error handling of |

I encountered several small issues with the printers applet.
I modified the
applet.jsin/usr/share/cinnamon/applets/printers@cinnamon.organd tested printing documents.The things I did correct:
cancel -afor a specific printer, even if all of the jobs on this printer belonged to him.-> Now the "Cancel all jobs" command tries to cancel every single job, not
cancel -afor every printer(job number) lpstat -o(lpstat several times, if there was more than one job - see point before)-> The description is now:
(job number) 'document_name' on <printer_name> (<size in Bytes/MB/KB>) by <user>(using lpq -a).The following problem could occur:If a job has the same number as byte-size like the job-id of the following one, the username and the filesize of the first one will be strange (job-id and functions stay working).I would say this will happen that seldom - we should ignore it.Which user will open the menu exactly in that minute and have a problem with a strange username or filesize?EDIT: fixed in new commit