Skip to content

Commit

Permalink
Improve icon loading to generate fallback icon if needed (#69)
Browse files Browse the repository at this point in the history
Details:
* This changes site loading/storing process to generate fallback icon from the site name when something is wrong with the site icon.
* It should allow user to install sites that may have broken icons.
* It does not apply to macOS yet.

Plans:
* In the future, `store_icon`/`store_icons` functions should be improved to fall back to the next available icon, and only generate icon if no icons work.
* This should also apply to macOS.
  • Loading branch information
filips123 committed Oct 29, 2021
1 parent ee2b831 commit f0cd6e4
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 31 deletions.
23 changes: 21 additions & 2 deletions native/src/integrations/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use anyhow::{Context, Result};
use data_url::DataUrl;
use glob::glob;
use image::imageops::FilterType::Gaussian;
use log::error;
use log::{error, warn};
use url::Url;
use web_app_manifest::resources::IconResource;
use web_app_manifest::types::{ImagePurpose, ImageSize};
Expand Down Expand Up @@ -254,7 +254,26 @@ pub fn install(info: &SiteInfoInstall, dirs: &ProjectDirs) -> Result<()> {
let exe = dirs.executables.join("firefoxpwa").display().to_string();

store_icons(&info.id, &info.name, info.icons, "")
.context("Failed to process and store site icons")?;
.context("Failed to process and store site icons")
.or_else(|error| -> Result<()> {
// Something is wrong with the site icon, so we should just log error and fallback to generated icon from the site name
// TODO: In the future we should improve `store_icons` to fall back to the next available icon, and only generate icon if no icons work
error!("{:?}", error);
warn!("Falling back to the generated icon from the site name");

let directory = directories::BaseDirs::new()
.context(BASE_DIRECTORIES_ERROR)?
.data_dir()
.join("icons/hicolor/48x48/apps");
let filename = directory.join(format!("FFPWA-{}.png", &info.id));

let letter = info.name.chars().next().context(GET_LETTER_ERROR)?;
create_dir_all(directory).context(CREATE_ICON_DIRECTORY_ERROR)?;
generate_icon(letter, 256, &filename).context(GENERATE_ICON_ERROR)?;

Ok(())
})?;

create_application_entry(info, &exe).context("Failed to create application entry")?;
let _ = update_application_cache();

Expand Down
67 changes: 38 additions & 29 deletions native/src/integrations/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use bindings::Windows::Win32::UI::Shell::{
use data_url::DataUrl;
use image::imageops::FilterType::Gaussian;
use image::GenericImageView;
use log::error;
use log::{error, warn};
use serde::de::Unexpected::Bytes;
use url::Url;
use web_app_manifest::resources::IconResource;
Expand All @@ -36,6 +36,21 @@ use winreg::RegKey;
use crate::directories::ProjectDirs;
use crate::integrations::{generate_icon, is_icon_supported, SiteInfoInstall, SiteInfoUninstall};

fn sanitize_name<'a>(name: &'a str, id: &'a str) -> String {
let pattern: &[_] = &['.', ' '];

let mut sanitized: String = name.chars().take(60).collect();
sanitized = sanitize_filename::sanitize(sanitized);
sanitized = sanitized.trim_end_matches(pattern).into();
sanitized = sanitize_filename::sanitize(sanitized);

if sanitized.is_empty() {
format!("Site {}", &id)
} else {
sanitized
}
}

fn store_icon(
siteid: &str,
iconid: &str,
Expand Down Expand Up @@ -120,7 +135,6 @@ fn store_icon(
Ok(icon)
}

#[inline]
fn create_arp_entry(info: &SiteInfoInstall, exe: String, icon: String) -> Result<()> {
let (key, _) = RegKey::predef(HKEY_CURRENT_USER)
.create_subkey(
Expand All @@ -141,27 +155,16 @@ fn create_arp_entry(info: &SiteInfoInstall, exe: String, icon: String) -> Result
Ok(())
}

#[inline]
fn create_menu_shortcut(info: &SiteInfoInstall, exe: String, icon: String) -> Result<()> {
unsafe {
// Limit the name and description to prevent overflows
let mut name: String = info.name.chars().take(60).collect();
let description: String = info.description.chars().take(240).collect();

// Sanitize the name to prevent invalid filenames
let pattern: &[_] = &['.', ' '];
name = sanitize_filename::sanitize(name);
name = name.trim_end_matches(pattern).into();
name = sanitize_filename::sanitize(name);
if name.is_empty() {
name = format!("Site {}", info.id);
}
// Sanitize the name to prevent overflows and invalid filenames
let name = sanitize_name(&info.name, &info.id);

// Set general shortcut properties
let link: IShellLinkW = windows::create_instance(&ShellLink)?;
link.SetPath(exe)?;
link.SetArguments(format!("site launch {}", info.id))?;
link.SetDescription(description)?;
link.SetDescription(info.description.chars().take(240).collect::<String>())?;
link.SetIconLocation(icon, 0)?;
link.SetShowCmd(7)?;

Expand Down Expand Up @@ -192,7 +195,6 @@ fn create_menu_shortcut(info: &SiteInfoInstall, exe: String, icon: String) -> Re
Ok(())
}

#[inline]
fn create_jump_list_tasks(info: &SiteInfoInstall, dirs: &ProjectDirs, exe: String) -> Result<()> {
unsafe {
let appid = format!("filips.firefoxpwa.{}", info.id);
Expand Down Expand Up @@ -290,12 +292,27 @@ fn create_jump_list_tasks(info: &SiteInfoInstall, dirs: &ProjectDirs, exe: Strin
Ok(())
}

#[inline]
pub fn install(info: &SiteInfoInstall, dirs: &ProjectDirs) -> Result<()> {
let _ = remove_dir_all(dirs.userdata.join("icons").join(&info.id));

let exe = dirs.executables.join("firefoxpwa.exe").display().to_string();
let icon = store_icon(&info.id, "site", &info.name, info.icons, dirs, false)
.context("Failed to process and store site icon")?;
.context("Failed to process and store site icon")
.or_else(|error| -> Result<String> {
// Something is wrong with the site icon, so we should just log error and fallback to generated icon from the site name
// TODO: In the future we should improve `store_icon` to fall back to the next available icon, and only generate icon if no icons work
error!("{:?}", error);
warn!("Falling back to the generated icon from the site name");

let directory = dirs.userdata.join("icons").join(&info.id);
let filename = directory.join("site.ico");

let letter = info.name.chars().next().context("Failed to get first letter")?;
create_dir_all(&directory).context("Failed to create icons directory")?;
generate_icon(letter, 256, &filename).context("Failed to generate icon")?;
Ok(filename.display().to_string())
})?;

windows::initialize_mta()?;

Expand All @@ -306,18 +323,10 @@ pub fn install(info: &SiteInfoInstall, dirs: &ProjectDirs) -> Result<()> {
Ok(())
}

#[inline]
pub fn uninstall(info: &SiteInfoUninstall, dirs: &ProjectDirs) -> Result<()> {
// Shortcut name is limited to prevent overflows
let mut name: String = info.name.chars().take(60).collect();

// Shortcut name is sanitized to prevent invalid filenames
let pattern: &[_] = &['.', ' '];
name = sanitize_filename::sanitize(name);
name = name.trim_end_matches(pattern).into();
name = sanitize_filename::sanitize(name);
if name.is_empty() {
name = format!("Site {}", info.id);
}
// Sanitize the name to prevent overflows and invalid filenames
let name = sanitize_name(&info.name, &info.id);

// Remove icons
let icon = dirs.userdata.join("icons").join(&info.id);
Expand Down

0 comments on commit f0cd6e4

Please sign in to comment.