Skip to content

Fix for missing Rich Link Preview - #4648

Closed
newhinton wants to merge 3 commits into
nextcloud:masterfrom
newhinton:master
Closed

Fix for missing Rich Link Preview#4648
newhinton wants to merge 3 commits into
nextcloud:masterfrom
newhinton:master

Conversation

@newhinton

Copy link
Copy Markdown
Contributor

Those commits should add the missing information for rich link previews in apps and social media link previews

This should add the correct Information to the rich link preview (Issue #4264)
This should add the correct Information to the rich link preview (Issue #4264)
@mention-bot

Copy link
Copy Markdown

@newhinton, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @jancborchardt and @juliushaertl to be potential reviewers.

@newhintonnewhinton mentioned this pull request May 1, 2017
<meta name="viewport" content="width=device-width, minimum-scale=1.0, maximum-scale=1.0">
<meta name="theme-color" content="<?php p($theme->getColorPrimary()); ?>">

<meta property="og:description" content="<?php p($theme->getTitle()); p(" - "); p($theme->getSlogan()); ?>"/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be escaped somehow, e.g. quotes in the title or slogan could mess up the HTML otherwise.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhm? p() already escapes :-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right… nvm

@jancborchardtjancborchardt added 3. to review Waiting for reviews bug labels May 2, 2017
<meta name="mobile-web-app-capable" content="yes">
<meta name="theme-color" content="<?php p($theme->getColorPrimary()); ?>">

<meta property="og:description" content="<?php p($theme->getTitle()); p(" - "); p($theme->getSlogan()); ?>"/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this in the layout.user.php file? This file will not be crawled by any of the services since it requires login, right?

<meta name="viewport" content="width=device-width, minimum-scale=1.0, maximum-scale=1.0">
<meta name="theme-color" content="<?php p($theme->getColorPrimary()); ?>">

<meta property="og:description" content="<?php p($theme->getTitle()); p(" - "); p($theme->getSlogan()); ?>"/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apps/files_sharing/lib/Controller/ShareController.php is already setting opengraph headers. Those should not be doubled. 😉

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@newhinton can you fix this? :)

@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 12, 2017
@rullzerrullzer added this to the Nextcloud 13 milestone May 23, 2017
@jancborchardt

Copy link
Copy Markdown
Member

@newhinton any update here? :) @ChristophWurst@juliushaertl maybe we should complete this fix so we get it in?

@juliusknorr

Copy link
Copy Markdown
Member

I'll check that out.

@juliusknorrjuliusknorr self-assigned this Jul 11, 2017
@newhinton

Copy link
Copy Markdown
ContributorAuthor

Sorry, but i dont have time to find out how to add those headers properly :/

@juliusknorr

Copy link
Copy Markdown
Member

Closing this in favour of #6342. Thanks @newhinton for the initial effort.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developingWork in progressbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@newhinton@mention-bot@jancborchardt@juliusknorr@LukasReschke@ChristophWurst@rullzer@MorrisJobke