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

fix(linux): Fix icon for .kmp files #11295

Merged
merged 1 commit into from
May 1, 2024
Merged

fix(linux): Fix icon for .kmp files #11295

merged 1 commit into from
May 1, 2024

Conversation

ermshiperete
Copy link
Contributor

It seems some Ubuntu versions ignore the icon field and only use generic-icon, so this change adds that field. This will fix displaying the icon on newer Ubuntu versions. And since we keep the icon field it should still work everywhere else.

Fixes #11144.

User Testing

Preparations

  • The tests should be run on these Linux platforms:

    • GROUP_NOBLE_WAYLAND: Ubuntu 24.04 Noble with Gnome Shell and Wayland
    • GROUP_WASTA: Wasta 20.04 with Cinnamon
  • Install build artifacts of this PR

  • Reboot

Tests

TEST_ICON:

  • Download a .kmp file from keyman.com
  • Verify that the correct icon shows for the .kmp file in the file manager

It seems some Ubuntu versions ignore the `icon` field and only use
`generic-icon`, so this change adds that field. This will fix displaying
the icon on newer Ubuntu versions. And since we keep the `icon` field
it should still work everywhere else.

Fixes #11144.
@ermshiperete ermshiperete added this to the B17S6 milestone Apr 24, 2024
@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Apr 24, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Apr 24, 2024

User Test Results

Test specification and instructions

  • ✅ GROUP_NOBLE_WAYLAND: Ubuntu 24.04 Noble with Gnome Shell and Wayland

    1 tests PASSED
    • TEST_ICON (PASSED): Tested with the attached PR build (17.0.14-beta (package version 17.0.314-1~PR-11295-2783.1+noble1) ) on Ubuntu Noble 24.04 Linux OS (VM) and here is my observation: 1. Downloaded Khmer_Angkor and SIL (IPA) keyboards. 2. Verified that the correct icon appears for the corresponding .kmp files in the file manager. FYI, it is showing a blank icon in the Firefox Download list. (notes)
  • ✅ GROUP_WASTA: Wasta 20.04 with Cinnamon

    1 tests PASSED
    • TEST_ICON (PASSED): Tested with the attached PR build (Keyman 17.0.314-1~PR-11295-2783.1+focal1) on Wasta 22.04 Linux OS and here is my observation: 1. Downloaded Khmer_Angkor and SIL (IPA) keyboards. 2. Verified that the correct icon appears for the corresponding .kmp files in the file manager. (notes)

Test Artifacts

Copy link
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

lgtm

@bharanidharanj
Copy link

GROUP_WASTA: Wasta 20.04 with Cinnamon

  • TEST_ICON (PASSED): Tested with the attached PR build (Keyman 17.0.314-1~PR-11295-2783.1+focal1) on Wasta 22.04 Linux OS and here is my observation: 1. Downloaded Khmer_Angkor and SIL (IPA) keyboards. 2. Verified that the correct icon appears for the corresponding .kmp files in the file manager.

@bharanidharanj
Copy link

Test Results

GROUP_NOBLE_WAYLAND: Ubuntu 24.04 Noble with Gnome Shell and Wayland

  • TEST_ICON (BLOCKED): 1. Got an error message while installed the attached PR build on Ubuntu Noble_Wayland Linux OS. 2. This error prevents further testing on this test.

@keymanapp-test-bot keymanapp-test-bot bot added user-test-failed and removed user-test-required User tests have not been completed labels Apr 25, 2024
@ermshiperete
Copy link
Contributor Author

ermshiperete commented Apr 26, 2024

@bharanidharanj What command do you use to install? I just tried with sudo apt install ./*.deb and that succeeded. Also, make sure you have all updates installed before trying to install Keyman (sudo apt update && sudo apt upgrade). Ubuntu 24.04 Noble got released yesterday - there have been a lot of changes in the last weeks leading up to the release.

@bharanidharanj
Copy link

@bharanidharanj What command do you use to install? I just tried with sudo apt install ./*.deb and that succeeded. Also, make sure you have all updates installed before trying to install Keyman (sudo apt update && sudo apt upgrade). Ubuntu 24.04 Noble got released yesterday - there have been a lot of changes in the last weeks leading up to the release.

@ermshiperete Here, What I did is: 1. Downloaded the PR build. 2. Opened it with terminal window. 3. Run the command to install the PR build and getting the above mentioned error message.

@darcywong00 darcywong00 modified the milestones: B17S6, A18S1 Apr 28, 2024
@ermshiperete
Copy link
Contributor Author

@bharanidharanj ah, ok. Found the problem. Please run:

sudo apt update
sudo apt upgrade
sudo apt dist-upgrade

then try the installation again.

@ermshiperete
Copy link
Contributor Author

@keymanapp-test-bot retest GROUP_NOBLE_WAYLAND TEST_ICON

@keymanapp-test-bot keymanapp-test-bot bot added user-test-required User tests have not been completed and removed user-test-failed labels Apr 30, 2024
@bharanidharanj
Copy link

Test Results

GROUP_NOBLE_WAYLAND: Ubuntu 24.04 Noble with Gnome Shell and Wayland

  • TEST_ICON (PASSED): Tested with the attached PR build (17.0.14-beta (package version 17.0.314-1~PR-11295-2783.1+noble1) ) on Ubuntu Noble 24.04 Linux OS (VM) and here is my observation: 1. Downloaded Khmer_Angkor and SIL (IPA) keyboards. 2. Verified that the correct icon appears for the corresponding .kmp files in the file manager. FYI, it is showing a blank icon in the Firefox Download list.

..icon appears correctly in the Downloads window

..blank icon appears in the Firefox Download list

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Apr 30, 2024
@mcdurdin mcdurdin merged commit 38a0359 into beta May 1, 2024
8 checks passed
@mcdurdin mcdurdin deleted the fix/linux/icon branch May 1, 2024 04:21
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.317-beta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(linux): Blank icon appears in the download folder after downloading a .kmp file from the Keyman site
5 participants