bootscore / bs-preloader

WordPress plugin to add a preloader to Bootscore theme. Content will be shown when the page is fully loaded.
https://bootscore.me/
MIT License
6 stars 3 forks source link

[BUG] Favicon disappears on non-admin pages #22

Open murilocarvalhodev opened 1 week ago

murilocarvalhodev commented 1 week ago

WordPress 6.6.2 Theme - Bootscore 6.0.1 Plugin - bsPreloader 5.4.1

I noticed that when keeping the bsPreloader plugin activated, the Favicon of pages that are not administrative disappears. This is not happening on your main site https://bootscore.me/. The main difference I notice is in the WordPress version. I believe yours is on 6.6.1 according to the Wappalyzer Google Chrome extension. Maybe the investigation can start from there...

Thank you guys, your work is wonderful! It's great to use Bootscore!

Captura de tela 2024-09-28 062839 Captura de tela 2024-09-28 062906

crftwrk commented 1 week ago

That's interesting...

Firstly, bootscore.me uses WP 6.6.2 as well. You can check Bootstrap, WooCommerce and WordPress versions in the site footer just above the copyright. So, it has nothing to do with this minor version, maybe since 6.6 because this was a major update.

There are several ways to add the favicons, in WP backend, in the WP Customizer, using a plugin or code it yourself. Question is how you did it and a link to your site would be nice to check.

bootscore.me uses a hook in the child-theme to add them in wp_head with a priority of 5, favicons generated by https://realfavicongenerator.net:

/**
 * Favicon & touch-icons
 */
function bootscore_favicon() {
  $output = '
    <link rel="apple-touch-icon" sizes="180x180" href="' . get_stylesheet_directory_uri() . '/assets/img/favicon/apple-touch-icon.png">
    <link rel="icon" type="image/png" sizes="32x32" href="' . get_stylesheet_directory_uri() . '/assets/img/favicon/favicon-32x32.png">
    <link rel="icon" type="image/png" sizes="16x16" href="' . get_stylesheet_directory_uri() . '/assets/img/favicon/favicon-16x16.png">
    <link rel="manifest" href="' . get_stylesheet_directory_uri() . '/assets/img/favicon/site.webmanifest">
    <link rel="mask-icon" href="' . get_stylesheet_directory_uri() . '/assets/img/favicon/safari-pinned-tab.svg" color="#0d6efd">
    <meta name="msapplication-TileColor" content="#ffffff">
    <meta name="theme-color" content="#ffffff">
  ';

  echo $output;
}
add_action('wp_head', 'bootscore_favicon', 5);

While the preloader hooks to wp_head with no priority:

https://github.com/bootscore/bs-preloader/blob/87e4d93ae563b3f44f1ceb5eb340024ff09e68f4/main.php#L131-L135

Just an idea, but trying to play around with the priority of the preloader?

murilocarvalhodev commented 1 week ago

I'm inserting the favicon through WP Customizer. The website is at testes.rumordigital.com.br His Local version is displaying the favicon normally. I already suspected the cache, but I've already cleared it in several places. I also installed the bsPreloader plugin in other projects that are using Bootscore and the favicon really disappears. The hosting server I'm using is LiteSpeed.

I'll try to change priorities. As a last resort, I resolve it manually until we find out. Thank you for your attention!

murilocarvalhodev commented 1 week ago
function bs_header_preloader() { 
   return bs_preloader_get_template('preloader.php'); 
 } 
 add_action('wp_head', 'bs_header_preloader',99); 

I stopped to take a closer look, and it was actually the change you suggested that resolved the problem. I added a higher priority to the hook and it resolved it. But that's not the recommended way to do it, right?

crftwrk commented 5 days ago

We can increase the priority if it's deeply tested. Never heard about this problem before and 99 seems a bit high? But what's about this:

Adding a filter to the action with priority for example of 10:

add_action('wp_head', 'bs_header_preloader', apply_filters('bootscore/bs-preloader/increase-priority', 10));

Change the priority in the child's functions.php:

// Add a filter that allows modification of the priority.
add_filter('bootscore/bs-preloader/increase-priority', function() {
  return 99; 
});
murilocarvalhodev commented 5 days ago

I just did the test, increasing it by 10 until I reached 90 and then went up to 98, and ironically, it only works when I reach 99.

https://github.com/user-attachments/assets/2b79348b-98ff-4880-aa83-c5d58dc15269

murilocarvalhodev commented 5 days ago

I tested the suggestion with the filter, and it doesn't work. Just passing 99 as the default value, then it doesn't make sense to use the filter for priority.

crftwrk commented 5 days ago

Quickly asked ChatGDP for this. Here is the answer:

In general, increasing the priority of an action in WordPress is not inherently a problem. However, it's important to understand the implications:

Higher priority value (e.g., 99): This means the function will be executed later in the order of all the functions hooked to wp_head. This could be beneficial if the preloader HTML should be one of the last things added to the , for example, if it relies on other elements already being present.

Potential issues:

Dependencies: If the preloader requires certain scripts, styles, or content to be added before it (e.g., jQuery or custom styles), you should ensure those assets are loaded before your preloader runs. Overlapping output: If other functions hooked to wp_head with lower priorities (executed earlier) also output content, they might interfere with your preloader if they depend on it being in place earlier. As long as the preloader is independent of other content in wp_head, increasing the priority should not cause any issues. Just be mindful of other things hooked into wp_head that might depend on the execution order.

So, we have to check this first and then changing to 99 in general.

crftwrk commented 3 days ago

I have tested priority 99 with multiple sites and different ways of adding the favicons and have not found any issues. In my opinion we can change this. @murilocarvalhodev want to create a PR? Would be great having you here.