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 Chrome/Chromium version detection #11

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

santuari
Copy link
Contributor

On RHEL 7, using Chromium PDF Export fails.
This is caused by the version of Chome Google Chrome 73.0.3683.103 which is different from Chromium Chromium 73.0.3683.86 Fedora Project.

It should mitigate icingaweb2-module-reporting issue #7.

@santuari
Copy link
Contributor Author

@Thomas-Gelf what do you think?

@lippserd
Copy link
Member

I'll have a look at that one. Thanks for the PR!

@lippserd lippserd self-assigned this Apr 29, 2019
@Thomas-Gelf
Copy link

@lippserd: instructions on how to install chromium (not chrome) might need improvement. This patch helps those running Chromium on CentOS/RHEL/Fedora as otherwise version detection will always fail.

@santuari: lgtm ;-)

@bsdlme
Copy link

bsdlme commented May 17, 2019

Can the path to the chrom(e|ium) binary be made user configurable trough the web interface? E.g. on FreeBSD Chromium binary is at /usr/local/bin/chrome, so at the moment I have to change the path in library/Pdfexport/ProvidedHook/Pdfexport.php.

@dnsmichi
Copy link

You can do that inside the module's config.ini already, it just lacks the config forms. Try something like this, IIRC:

[chrome]
binary = "..."

@lippserd lippserd changed the title fix version getter for chromium Fix Chrome/Chromium version detection Jun 7, 2019
@lippserd lippserd merged commit 1f3f834 into Icinga:master Jun 7, 2019
@lippserd lippserd added this to the 0.9.1 milestone Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants