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

Properly replace open graph favicon #5322

Closed
LukasReschke opened this issue Jun 9, 2017 · 3 comments
Closed

Properly replace open graph favicon #5322

LukasReschke opened this issue Jun 9, 2017 · 3 comments

Comments

@LukasReschke
Copy link
Member

if ($shareTmpl['previewSupported']) {
$shareTmpl['previewImage'] = $this->urlGenerator->linkToRouteAbsolute( 'files_sharing.PublicPreview.getPreview',
['x' => 200, 'y' => 200, 'file' => $shareTmpl['directory_path'], 't' => $shareTmpl['dirToken']]);
} else {
$shareTmpl['previewImage'] = $this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath('core', 'favicon-fb.png'));
}
is not replacing the OpenGraph favicon so when sharing to Twitter, Facebook or some other place parsing it the icon will still show the Nextcloud logo.

We should probably use \OCA\Theming\Controller\IconController::getTouchIcon here if available.

@LukasReschke LukasReschke added this to the Nextcloud 12.0.1 milestone Jun 9, 2017
LukasReschke added a commit that referenced this issue Jun 9, 2017
Fixes #5322

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
LukasReschke added a commit that referenced this issue Jun 9, 2017
Fixes #5322

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@JaredBoone
Copy link
Contributor

@MorrisJobke I'd like favicon-fb.png to be overridden by the favicon specified in the Theming app. It's not currently the case (as of 15.0.2). Who should I talk to about that and how can I help? Thanks!

@MorrisJobke
Copy link
Member

cc @juliushaertl

@JaredBoone Could you also open a separate ticket for this. It seems the old icon is used there as well for untamed instances.

@juliushaertl
Copy link
Member

@JaredBoone Yes, please open a new ticket for this. If you want to look into fixing this, the relevant code part where the icons are replaced is here: https://github.com/nextcloud/server/blob/master/apps/theming/lib/ThemingDefaults.php#L337-L342

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

No branches or pull requests

5 participants