Fix menu launcher position and widget icons #17
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DemiMarie
reviewed
Jul 14, 2023
kde-settings-qubes.spec.in
Outdated
Comment on lines
129
to
144
| touch --no-create %{_kde4_iconsdir}/hicolor &> /dev/null ||: | ||
| touch --no-create %{_kde4_iconsdir}/breeze &> /dev/null ||: | ||
|
|
||
| %posttrans | ||
| gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null ||: | ||
| gtk-update-icon-cache %{_kde4_iconsdir}/breeze &> /dev/null ||: | ||
| update-desktop-database -q &> /dev/null ||: | ||
|
|
||
| %postun | ||
| if [ $1 -eq 0 ] ; then | ||
| touch --no-create %{_kde4_iconsdir}/hicolor &> /dev/null ||: | ||
| touch --no-create %{_kde4_iconsdir}/breeze &> /dev/null ||: | ||
| gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null ||: | ||
| gtk-update-icon-cache %{_kde4_iconsdir}/breeze &> /dev/null ||: | ||
| update-desktop-database -q &> /dev/null ||: | ||
| fi |
There was a problem hiding this comment.
Suggested change
| touch --no-create %{_kde4_iconsdir}/hicolor &> /dev/null ||: | |
| touch --no-create %{_kde4_iconsdir}/breeze &> /dev/null ||: | |
| %posttrans | |
| gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null ||: | |
| gtk-update-icon-cache %{_kde4_iconsdir}/breeze &> /dev/null ||: | |
| update-desktop-database -q &> /dev/null ||: | |
| %postun | |
| if [ $1 -eq 0 ] ; then | |
| touch --no-create %{_kde4_iconsdir}/hicolor &> /dev/null ||: | |
| touch --no-create %{_kde4_iconsdir}/breeze &> /dev/null ||: | |
| gtk-update-icon-cache %{_kde4_iconsdir}/hicolor &> /dev/null ||: | |
| gtk-update-icon-cache %{_kde4_iconsdir}/breeze &> /dev/null ||: | |
| update-desktop-database -q &> /dev/null ||: | |
| fi | |
| touch --no-create %{_kde4_iconsdir}/hicolor > /dev/null 2>&1 ||: | |
| touch --no-create %{_kde4_iconsdir}/breeze > /dev/null 2>&1 ||: | |
| %posttrans | |
| gtk-update-icon-cache %{_kde4_iconsdir}/hicolor > /dev/null 2>&1 ||: | |
| gtk-update-icon-cache %{_kde4_iconsdir}/breeze > /dev/null 2>&1 ||: | |
| update-desktop-database -q > /dev/null 2>&1 ||: | |
| %postun | |
| if [ $1 -eq 0 ] ; then | |
| touch --no-create %{_kde4_iconsdir}/hicolor > /dev/null 2>&1||: | |
| touch --no-create %{_kde4_iconsdir}/breeze > /dev/null 2>&1 ||: | |
| gtk-update-icon-cache %{_kde4_iconsdir}/hicolor > /dev/null 2>&1||: | |
| gtk-update-icon-cache %{_kde4_iconsdir}/breeze > /dev/null 2>&1 ||: | |
| update-desktop-database -q > /dev/null 2>&1 ||: | |
| fi |
&> is a bashism
There was a problem hiding this comment.
Well, this is from the official Fedora packaging guidelines... But changing it makes sense anyway.
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.2&build=2023080409-4.2&flavor=pull-requests New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.2&build=2023071104-4.2&flavor=update
Failed tests13 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/77326#dependencies 17 fixed
Unstable tests
|
marmarek
force-pushed
the
menu-position
branch
2 times, most recently
from
e44680b to
545a098
Compare
Apparently there is no other API method than settings AppletOrder directly. This may require restarting panel after applying the change - it seems like changing it on the live panel doesn't work. Furthermore, on initial start the AppletOrder may be not set yet, in that case get initial value from widgetIds property. QubesOS/qubes-issues#8159
KDE stores list of updates already applied, by name. Since 10-qubes.js was changed, rename it so the new version is applied too. QubesOS/qubes-issues#8159
Otherwise GTK will be confused about older cache and icons won't work in GTK-based widgets. Related to QubesOS/qubes-issues#2968
Besides being unused, it overrides panels() function. While it isn't used in this script, it isn't clear (to me) what scope does it have, and it might be relevant to other scripts.
DemiMarie
reviewed
Aug 2, 2023
kde-settings-qubes.spec.in
Outdated
Comment on lines
120
to
126
| %triggerin -- plasma-desktop | ||
| default_layout="/usr/share/plasma/layout-templates/org.kde.plasma.desktop.defaultPanel/contents/layout.js" | ||
| if ! grep -q qubesMenu "$default_layout"; then | ||
| cp -a "$default_layout" "$default_layout.qubes-orig" | ||
| sed -i \ | ||
| -e "s#.*org.kde.plasma.kickoff.*#var qubesMenu = panel.addWidget('org.kde.plasma.quicklaunch')\nqubesMenu.currentConfigGroup = ['General']\nqubesMenu.writeConfig('launcherUrls', ['file:///usr/share/applications/open-qubes-app-menu.desktop'])#" \ | ||
| -e "s/^kickoff/qubesMenu/" \ | ||
| "$default_layout" | ||
| fi |
There was a problem hiding this comment.
Using sed on a JavaScript file??? On the one hand this is a disgusting hack, but on the other hand the proper way to do this would be to use a JavaScript parser and I don’t know of one that is suitable for this.
There was a problem hiding this comment.
This is an alternative to shipping own copy of the whole file, not to a full parser. I could use a patch, but honestly, it doesn't matter that much in this place.
There was a problem hiding this comment.
A patch would make it much easier to understand what is going on here. That said I think shipping the full file would be best, especially for R4.2 users who are still getting dom0 updates from Fedora.
There was a problem hiding this comment.
still getting dom0 updates from Fedora.
That's exactly the reason why I do not want to ship the whole file - I only want to replace the menu, while keeping upstream layout of the rest of the panel.
The layout update script is running too early on the first KDE startup, and default widgets are not there yet. This ends up in menu launcher remaining on the right, instead of the left. QubesOS/qubes-issues#8159
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
QubesOS/qubes-issues#8159
QubesOS/qubes-issues#2968