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

Don't output meta tags with blank content #4424

Closed
novaugust opened this issue Nov 10, 2014 · 13 comments · Fixed by #8150
Closed

Don't output meta tags with blank content #4424

novaugust opened this issue Nov 10, 2014 · 13 comments · Fixed by #8150
Labels
bug [triage] something behaving unexpectedly

Comments

@novaugust
Copy link
Contributor

If a user doesn't enter anything for their post's meta description, the meta description tag is still output but with an empty content, like: <meta name="description" content="">

If the content is empty, the tag should never be output in the first place.

Ref this comment

@novaugust novaugust added the good first issue [triage] Start here if you've never contributed before. label Nov 10, 2014
@novaugust novaugust added this to the Future Backlog milestone Nov 10, 2014
@ErisDS
Copy link
Member

ErisDS commented Nov 10, 2014

Hey @novaugust this is unfortunately not a simple issue to fix, I was working on raising it but haven't figured out what to do yet. This is non-trivial to fix because the meta_description tag is used like this:

<meta name="description" content="{{meta_description}}" />

We need to either make it possible to check if a helper has a value or move the output of the meta title and description into {{ghost_head}} and possibly both.

Making it possible to check a helper is tricky because it means either coming up with some sort of helper to do it like {{#has_value}} (but naming things is hard) OR modifying the behaviour of the {{#if}} helper.

Moving the output of meta tags/description into ghost_head isn't so tricky, but means that there will be an overlap where most themes will have duplicate tags, which requires a bit of research into what kind of impact that will have.

We could also change the {{meta_description}} helper to output the full HTML, and only output if there is something to output, however that would be even harder to manage in terms of the upgrade path.

@ErisDS ErisDS removed the good first issue [triage] Start here if you've never contributed before. label Nov 10, 2014
@javorszky
Copy link
Contributor

What if new helper: if_not_empty, that returns true for... well, strlen( thing ) > 0. And then

{{#if_not_empty meta_description}}
    <meta name="description" content="{{meta_description}}" />
{{/if_not_empty}}

@javorszky
Copy link
Contributor

Actually, would an empty string even trigger an `{{#if}}``?

EDIT: no. If {{meta_description}} is empty, then {{#if}} doesn't trigger. :) Sauce: http://tryhandlebarsjs.com/

So basically:

{{#if meta_description}}
    <meta name="description" content="{{meta_description}}" />
{{/if}}

@novaugust
Copy link
Contributor Author

@javorszky I think the problem is, meta_description is a helper, not a string (why is this?).
I like the idea of messing with ghost_head to not use the theme api to build this sort of stuff. It should just inject it direct-o-lee?
... I need to look at this code before I comment more :P

@javorszky
Copy link
Contributor

@novaugust aren't helpers are of the syntax {{#foo}} where foo is the name of the helper? But even if meta_description is a helper, it returns a string. Therefore it's either false/empty or string of length > 0, no?

@novaugust
Copy link
Contributor Author

Like I said, I need to look in the code to see what's actually going on.
But, if it's a helper, I don't believe you can just pass it to another
helper like that. We could probably do {{#if (meta_description)}}, if I
have my syntax right..
But anyways, no, you can get helpers without it being block helpers. See
for instance the author helper in the themes:
http://themes.ghost.org/v0.5.3/docs/author
If it was just data, we could do {{author.bio}}, but since it's a helper,
you can't reach into properties like that. You've got to do
{{#author}}{{bio}}{{/author}}

On Mon, Nov 10, 2014 at 1:26 PM, Gabor Javorszky [email protected]
wrote:

@novaugust https://github.com/novaugust aren't helpers are of the
syntax {{#foo}} where foo is the name of the helper? But even if
meta_description is a helper, it returns a string. Therefore it's either
false/empty or string of length > 0, no?


Reply to this email directly or view it on GitHub
#4424 (comment).

@joeldrapper
Copy link

A description meta element has nothing other than name and content attributes. There is nothing else to set or change about the element. Since by definition, the description meta element's name is, "description", that leaves only one variable, the content and therefore only two options:

  1. A meta description element with content
  2. No meta description

Since we've decided to put the option to set or turn off (by leaving blank) the meta description inside Ghost UI, I don't see why or what advantage we gain from allowing theme developers to mess around with it? i.e.

{{#if meta_description}}
   <meta name="description" data-douche="true" content="I'm a douche and I'm screwing up the recommended character limit. {{meta_description}}" />
{{/if}}

Besides, the W3 spec says "The value must be a free-form string that describes the page." and there's no sensible method or reason to disable the meta options in Ghost UI and set them to fixed values, unique to every page from within the theme.

If people really want to do something like this code example, I don't see why someone couldn't make a Meta Description Prepender & Appender App! On a serious note though, someone who thinks they can write a better algorithm than Google for detecting and summarising what a page is about could well make an App that fills in the blanks if the user leaves it empty. I'm sure all the SEO-mad bloggers would love it.

I'd say put it in ghost_head. Just my two cents.

@Kikobeats
Copy link

hey guys, some problem here.

I understand that the first purpose for meta_description is use it in combination with html meta tag, but basically if you don't set the {{meta_description}} of your post, is in blank (in the post preview you have a preview using the first paraph but actually the variable is empty!)

so, IMHO, basically the right way should be, if the variable is empty, get the html content of the first paraph like the preview.

Use {{excerpt}} instead of {{meta_description}} when is blank doesn't fix the problem because is not possible to access in the context.

I read that moz says that is better uses empty string that random text. maybe something intermediate like use the first the of the post since the first dot? lol

@JohnONolan
Copy link
Member

So: outputting an empty meta description is definitely completely wrong. We shouldn't be doing that. However: outputting any random content as a custom meta description is actually worse for SEO, not better. If you don't enter a meta description, Google will choose the most relevant content to the search term on the page to display as the meta description - which is far better. More info: http://moz.com/learn/seo/meta-description

One option is what @javorszky said:

{{#if meta_description}}
    <meta name="description" content="{{meta_description}}" />
{{/if}}

however I'm intrigued by @joeldrapper's suggestion. He's actually right, I can't think of a compelling reason why meta description needs to be in the theme rather than in {{ghost_head}} - though the same could potentially be argued for <title>

Other than: It's slightly un-obvious so maybe theme authors would end up duplicating it cause they don't understand why it's not there? Not sure.

@joeldrapper
Copy link

@JohnONolan If an app or setting could optionally append - {{@blog.title}} (as is the custom of most websites) to the end of the title tag, it probably should be in {{ghost_head}} too.

I don't think theme authors would duplicate the tags if it was made very clear in the theme documentation what was included in ghost_head but it would effect current themes.

@ErisDS
Copy link
Member

ErisDS commented Apr 20, 2015

This wormed it's way into my brain again recently.

however I'm intrigued by @joeldrapper's suggestion. He's actually right, I can't think of a compelling reason why meta description needs to be in the theme rather than in {{ghost_head}} - though the same could potentially be argued for <title>

We could certainly output both the title and description in the {{ghost_head}} helper. There are a few small downsides:

  • current themes would end up with duplication, which is possibly undesirable?
  • we'd need to consider customisation options (should it be opt-outable?)
  • if we do this, why do we have a meta_title and meta_description helper anyways?

None of these are particularly terrible, they just make things a tad more complicated than they would otherwise be, so I thought up some other options:

I'm not a fan of the idea of modifying the {{#if}} helper and so far we've kept to this, opting to build our own rather than overriding the defaults. So instead, I've got 2 possible helper changes that could solve the problem:

1. Use the {{#has}} helper... it would require an update, but one that would be useful in other situations.

Either (ugly - not a fan)

{{#has meta_description="true"}}
    <meta name="description" content="{{meta_description}}" />
{{/has}}

Or (swap any for an attribute name of your choice, could be property or attribute or value or even notEmpty)

{{#has any="meta_description"}}
    <meta name="description" content="{{meta_description}}" />
{{/has}}

The nice thing about this idea is that there plenty of other use-cases for checking if a particular piece of data is available.

{{#has any="tags, feature"}}
 ... this post has either got at least one tag, or it is featured
{{/has}}

With this change, you would wrap the meta tags in Casper and other themes would soon follow suit.

2. Change the {{meta_description}} helper

We could change the meta helpers to have an optional property (again the name is not important just trying to get across a concept):

{{meta_description html="true"}}

This tells the helper to output the full HTML, and the logic for outputting the full HTML would be to only output the description if it was present. We could add the same property to meta_title, although it is less relevant.

Again this change would allow us to make a useful update to casper and let everyone else follow suit without causing any issues.

My favourite is adding some sort of property detection to the {{#has}} helper, as that makes sense in multiple use cases, but I'm not opposed to any of the other solutions. Getting to a clear decision would be good as most of these are quite easy to implement.

@JohnONolan
Copy link
Member

I'm very in favour of moving the whole thing into ghost_head.

The helpers are still relevant - Theme authors may want to output meta content in other places - but I guess that could be a data helper instead of a custom one or whatever

@ErisDS ErisDS modified the milestone: Future Backlog Oct 9, 2015
@ErisDS ErisDS added the bug [triage] something behaving unexpectedly label Oct 9, 2015
@ErisDS ErisDS mentioned this issue Feb 18, 2016
7 tasks
@ErisDS ErisDS added the themes label Sep 19, 2016
@damianmuti
Copy link

Was this feature added? I'm trying to find a way to print a page's meta_description if it is present in the page settings.

@kirrg001 kirrg001 mentioned this issue Jan 17, 2017
14 tasks
ErisDS added a commit to ErisDS/Ghost that referenced this issue Mar 14, 2017
closes TryGhost#4424

- meta description is an optional SEO tag that we can provide when we have sensible output
- in the cases where we have no useful output, we should not output the tag at all
- ghost_head now takes care of this, and themes should not include their own meta description tag
kirrg001 pushed a commit that referenced this issue Mar 14, 2017
closes #4424

- meta description is an optional SEO tag that we can provide when we have sensible output
- in the cases where we have no useful output, we should not output the tag at all
- ghost_head now takes care of this, and themes should not include their own meta description tag
ErisDS added a commit to TryGhost/Casper that referenced this issue Mar 14, 2017
refs TryGhost/Ghost#4424

- as  of TryGhost/Ghost#8150, `{{ghost_head}}` will output the meta description if Ghost is able to determine a sensible value to output.
kirrg001 added a commit to TryGhost/Casper that referenced this issue Mar 14, 2017
refs TryGhost/Ghost#4424

- as of TryGhost/Ghost#8150, `{{ghost_head}}` will output the meta description if Ghost is able to determine a sensible value to output
kirrg001 pushed a commit to TryGhost/Casper that referenced this issue Mar 14, 2017
refs TryGhost/Ghost#4424

- as  of TryGhost/Ghost#8150, `{{ghost_head}}` will output the meta description if Ghost is able to determine a sensible value to output.
baldwinjj pushed a commit to baldwinjj/Casper that referenced this issue Apr 6, 2017
refs TryGhost/Ghost#4424

- as  of TryGhost/Ghost#8150, `{{ghost_head}}` will output the meta description if Ghost is able to determine a sensible value to output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants