Defined App Image Icon isn't used

I have the same problem with the icon as with the pwa.
Drupal 10.1.0
PWA: 2.0.0-rc2

I managed to fix it by doing the following:
1.- Downgrade to version 9.5.9 of the core and version 8.x-1.7 of pwa
2.- I configure the hosting so that the web loads on the url https://dominio.com/ (before I had it in https://dominio.com/web)
I really believe that the fundamental factor is point 2

For the PWA and the APP installation to work you have to load the manifest.json manually at the link yoursite.com/manifest.json .

It's a bug in the main module, it doesn't load the file automatically.

If you have loaded the logo before drupal version 10.1.1 load the manifest.json file manually and it will work.

In drupal 10.1.1 it is not possible to add the logo of the app, it gives an error and the PWA module cannot be used. I have opened an issue.

jincy_k → closed merge request !59

updated image file path with drupal service to fetch the correct path

Thank you very very much @jincy_k great work! I guess we should add a test to ensure this doesn't ever happen again?
Also see 📌 Test the module and overhaul its code in the process. Fixed

After update from issue:

$wrapper = $this->streamWrapperManager->getViaScheme(
$this->config('system.file')->get('default_scheme')
);
- $files_path = '/' . $wrapper->basePath() . '/pwa/';
+ $files_path = \Drupal::service('file_url_generator')->generateString("public://pwa") . '/';
$file_uri = $files_path . $file->getFilename();

- $file_path = $wrapper->realpath() . '/pwa/' . $file->getFilename();
+ $file_path = $wrapper->realpath() . '/' . $file->getFilename();

$config->set('image', $file_uri)->save();

I have this error en browser logo of developer state:

The FetchEvent for "https://***.com/manifest.json" resulted in a network error response: a redirected response was used for a request whose redirect mode is not "follow".

And in drupal logo:

TypeError: imagecopyresampled(): Argument #2 ($src_image) must be of type GdImage, bool given in imagecopyresampled() (line 424 of /home/***/public_html/***.com/web/modules/contrib/pwa/src/Form/ManifestConfigurationForm.php).

manifest.json still not loading.

There are also problems with deleting This image is your application icon. (png files only, format: (512x512), when add or delete icon.

Thanks for help.

As äf #12

@ Anybody → , Many thanks. I totally agree with you, but due to my present hectic schedule I am unable to get involved. If anyone could help, please take this into consideration.
@ ady1503 → I'm not able to reproduce all of the issues listed. I'm testing locally (localhost without a valid ssl certificate). I'm getting this warning while deleting the image
(Warning: unlink(/app/web/web/sites/default/files/pwa/pwa_sample_1.png): No such file or directory in Drupal\pwa\Manifest->deleteImage() (line 158 of /app/web/modules/contrib/pwa/src/Manifest.php) in drupal dblog

Solution : check if the file exist in the path before unlink. I've tested this code locally and got this warning fixed
if (file_exists($path)) {
unlink($path);
}
I'm seeing an issue in the path,'web' repeating twice in the dblog : (Warning: unlink(/app/web/web/sites/default/files/pwa/pwa_sample_1.png) which is again due to getcwd(). I wish I could fix this too. I'm trying..

I have found that causes the error:

TypeError: imagecopyresampled(): Argument #2 ($src_image) must be of type GdImage, bool given in imagecopyresampled() (line 424 of /home/***/public_html/***.com/web/modules/contrib/pwa/src/Form/ManifestConfigurationForm.php).

If the option that Drupal controls and serves the files is activated, it gives an error. If it is chosen that the web server does it, it does not give an error.

I think in this code is the error:
line 401
// Save new image.
$wrapper = $this->streamWrapperManager->getViaScheme(
$this->config('system.file')->get('default_scheme')
);
Because if I have the file system private, it looks in the folder declared private in settings, but by default the PWA saves the files in public.

Have need to add an if function to check the filesystem and allocate the correct one.

Another very important thing is to delete the cookies from the website. Not only the Drupal cache, to see the changes in the manifest.json file.

It also remains that manifest.json file does not load automatically.

Gracias

@ady1503 thanks, could you prepare a MR basted on your results?

In the end I have managed to find the problems that the manifest.json file does not load automatically.

First important point is that web browsers other than "Chromium" group does not work the PWA. In Firefox, the manifest.json and the block appear, but the add APP button does not appear, in Opera the add APP button appears but nothing does when clicked.

Second point for anonymous users to see the PWA block and for application to be available on the web page, you need to have active: "See published content" for all roles and for the anonymous role.

I think you need to add a permission group to the PWA. For example by roles: view PWA, view PWA block, etc. The issue of permissions has to be prioritized. I have a mechanism that controls who can see what is published, but for simpler cases it will not be correct. PWA has to be controlled by roles.

Another thing that would be important is the name of the file, instead of manifest.json, many sources on the web recommend:

NOTE: File extension: .webmanifest or .json?
The IANA registered file extension for the manifest is .webmanifest. Some web servers recognize this extension and transfer the file using the standardized application manifest media type (application/manifest+json). Developers can also choose a different extension (e.g. .json) or none at all (e.g. /api/GetManifest), but are encouraged to transfer the manifest using the application/manifest+json MIME type, although any JSON MIME type is ok.

With these changes, I get a decent PWA functionality in my project.
If my thoughts are correct add to the following version.

Gracias

@ ady1503 → Through a simple workaround in the code, I am able to fix this problem when I select the private file system, see below :

In pwa/src/Form/ManifestConfigurationForm.php
$files_path = \Drupal::service('file_url_generator')->generateString("public://pwa") . '/';
$file_uri = $files_path . $file->getFilename();
$file_path = '/app'.$files_path . $file->getFilename();
$config->set('image', $file_uri)->save();

@ Anybody → This may not be a perfect solution, though.

Given that the issue, https://www.drupal.org/project/pwa/issues/3313423 🐛 Introduce the ability to render the PWA App Icon through the private file system Closed: won't fix - PWA App image does not render on a private file system exists, the proposed workaround can be taken as a temporary solution.

As long as schema set is "Public" for the upload, at line 305 of the same file,
'#upload_location' => 'public://pwa/',
it can always return $files_path, public://image.

If I'm making the wrong assumptions, kindly correct me.

Please see my latest commit to fix the issues
https://git.drupalcode.org/project/pwa/-/merge_requests/60/diffs?commit_...

@jincy_k thank you - could you please remove the comments and fix the code style issues?

Even if it fixes the issue, it doesn't seem to be correct to completely leave out the file system choice, but someone has to take a deeper look. I don't have much time for this, currently. sorry. Happy to review again, once this is carefully reviewed by any Drupal expert.

This issue seems quite cluttered and hard to navigate through. But the main issue pointed out here is probably caused by 🐛 Introduce the ability to render the PWA App Icon through the private file system Closed: won't fix , which @jincy_k already commented.

Important notice from that issue:

I'm getting this warning while deleting the image
(Warning: unlink(/app/web/sites/default/files/pwa/pwa_sample_1.png): No such file or directory in Drupal\pwa\Manifest->deleteImage() (line 158 of /app/web/modules/contrib/pwa/src/Manifest.php) in drupal dblog

This seems really annoying...

Let's change the title to reflect the MR proposed here.

The "image" defined in the setting isn't actually used anywhere in code. Instead, the app icons stored in /assets are put into the manifest.json and used. Meaning, the 'image' setting doesn't even have any impact on the app icon.

We either use the icon stored in the assets folder OR we use the default site logo if 'default_image' is checked. See 'getCleanValues()' in '/src/Manifest.php'.

Grevil → closed merge request !63

Updated the 'Steps to reproduce' section.

Partially moved, applied changes to 🐛 Introduce the ability to render the PWA App Icon through the private file system Closed: won't fix and reset issue branch to 2.x-dev.

While we are at it, we should overhaul the image field code, as it currently isn't best practice. And have three upload fields to upload all three icon sizes needed for the manifest.json.

@Grevil: Still on it?

Yes, once the other big issues are resolved. I prefer to not have to rebase that amount of code! 😅

Grevil → closed merge request !60

Ok, I think my changes in "3371404-defined-app-image-icon-is-not-used" are probably not the correct approach, since we are resizing the icon based on the original image supplied. So there is no need for having multiple image fields.

I'll continue on Wednesday.

@Grevil: Looks good so far. I think for the next steps it might make sense:

1. To create a separate managed file for each derivate, so it can be used easily and has the required APIs directly available for you, for example to get the paths
2. Store the fid (or whatever makes sense) as config value separately (or as array - what's easier for you)
3. When uploading a new image, remember to first set the old files deletable (by their id from the config) and then replace the config values by the new IDs

Grevil → closed merge request !81

Done!

Temporary providing the changes as a patch, because of internal technical issues.

@Grevil: Do you see any chance to put some tests for this in place? At least simple ones?

LGTM!

Anybody → closed merge request !80

Anybody → reopened merge request !80

Ehm, tugboat will only pick up the changes done inside the MR, which VASTLY differ from the patch provided.
(Although since Drupal changes to Gitlab CI, it might not work at all anymore).

Please ignore the MR (or close it) and only review the patch!

@Grevil: Do you see any chance to put some tests for this in place? At least simple ones?

File tests usually require a lot of setup, but I surely can if needed! (Although I don't think we need them, at least not simple ones)

@Grevil: Totally clear, was just a preparation for the next push expected. I won't review the patch.

RTBC was accidentally

Trying to get tugboat running for testing here...

Alright, MR is updated! Please review now. :)

GREAT work, thank you @Grevil all looking good!

Please add a point to a test issue, that the image upload, replace and deletions should be tested!
I think we shouldn't do it here and now due to missing ressources?

@Anybody, I agree!

I created a test issue here: 📌 List of tests to ensure the module's functionality Active .

Forgot to delete the old image paths on update hook!

Inside image, image_small and image_very_small, there used to be paths to the image instead of the file ID. Going to fix that in 📌 Remove old image paths Needs review .

Automatically closed - issue fixed for 2 weeks with no activity.