Skip to content

[#33487] Basic JMicrodata implementation in com_content (single article view), also JMicrodata implementation in plugin Content - Vote#3330

Closed
alexprut wants to merge 5 commits intojoomla:3.3-devfrom
alexprut:microdata-implementation-com_content
Closed

[#33487] Basic JMicrodata implementation in com_content (single article view), also JMicrodata implementation in plugin Content - Vote#3330
alexprut wants to merge 5 commits intojoomla:3.3-devfrom
alexprut:microdata-implementation-com_content

Conversation

@alexprut
Copy link
Copy Markdown

A basic implementation of JMicrodata library in the CMS , implemented the library in com_content (single article view) and in plugin Content - Vote.

Testing instructions (todo list)

Backend
  1. Administration (backend panel) → System → Global Configuration (Site tab) → SEO Settings →
    you should find a toggle button to enable/disable globally microdata semantics
  2. Administration (backend panel) → Content → Article Manager → Options (button) → Articles (tab) → you should find a toggle button to enable/disable microdata semantics globally in com_content
  3. Administration (backend panel) → Content → Article Manager → Edit (edit any article) → Options (tab) → you should find a toggle button to enable/disable microdata semantics in the current article, you should also find a dropdown list for choosing the microdata type
Frontend
  1. Please make sure you have you have enabled microdata from the administration backend panel (from the previous step).
  2. You should have enabled the Content - Vote plugin.
  3. Administration (backend panel) → Content → Article Manager → Edit (edit any article) → Options (tab) → Show Voting (dropdown list) → select Show
  4. Live Site (frontend) → Single Article (single article view, article from previous step in which you've enabled microdata) → Open the page source code (in firefox ctrl+u) → you shuould see microdata semantics in the html code
  5. Copy and paste the source in http://www.google.com/webmasters/tools/richsnippets

You should play around to see if everything works fine, if you have problems or difficulties in testing let me know, I will help you.

Small 'problems/questions'
  1. Should be or not a global configuration toggle button to enable/disable microdata semantics?
  2. In com_content, where should stay the enable/disable toggle button? in Options (tab)?

If this implementation works fine and the feedback is OK, I start with other PRs in order to finish the implementation of JMicrodata in com_content and other components/modules/plugins.
Thanks.

JTracker http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33487&start=0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we mark this as (CMS) 3.3 rather than 13.1 please?

@betweenbrain
Copy link
Copy Markdown
Contributor

I agree with @phproberto at #3252 (comment) about adding more parameters. Could always rendering the microdata ever harm a site?

@wilsonge
Copy link
Copy Markdown
Contributor

Agreed about the on/off. I guess we need the microdata type param though?? i'm not too clued up about it

@mbabker
Copy link
Copy Markdown
Contributor

mbabker commented Mar 19, 2014

A page can be one of a few different types. You'd need a way to define this type, especially as the different types can have different metadata.

@alexprut
Copy link
Copy Markdown
Author

@wilsonge
— Could you please confirm I should or not mark as (CMS) 3.3 rather than 13.1?
— For the conflicts with Roberto's JLayouts PR there is no big problem, I can fix it after.
— Take a look here to understand why we need a microdata Type http://schema.org/docs/full.html , this ways you can change the Type of the article/page.

@betweenbrain
— No it does not harm a site, but if you use it improperly can harm the website SEO, anyway if you use Joomla in a local net (work/home network) usually you don't need to use Microdata semantics, in most cases Microdata is used by search engines to display rich snippets, which improve the website SEO.

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 19, 2014

Could you please confirm I should or not mark as (CMS) 3.3 rather than 13.1?

There is no platform anymore. So 3.3 would be correct.

@javigomez
Copy link
Copy Markdown
Contributor

Regarding:

Administration (backend panel) → Content → Article Manager → Edit (edit any article) → Options (tab) → Show Voting (dropdown list) → select Show

Did you finally decided not to move it to "metadata" tab?

@alexprut
Copy link
Copy Markdown
Author

@javigomez Do you mean the Publishing (tab)?
Well for the moment the buttons are in the Options (tab) , but it was decided by me, so lets decide together where to place it :)

Administration (backend panel) → Content → Article Manager → Edit (edit any article) → Options (tab) → Show Voting (dropdown list) → select Show

the dropdown is not part of the implementation, it was allready there and used to show the content rating, but as I implemented the JMicrodata library in that plugin, it must be enabled for testing

@parthlawate
Copy link
Copy Markdown
Contributor

Tested .. All good !

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 19, 2014

You set the default value for the microdata param to be enabled and the type to "article" in the XML definitions. But when you retrieve the parameter value you don't set any default values. The result is that microdatas aren't enabled till the user opens and saves the params, and then it magically will be enabled without the user doing something. The same is true for the microdata type. As long as the article isn't saved, the type will be "thing". As soon as the article is saved again, it will take the default "article" type.

The default values used in the XML definition and the ones used in the code ($params->get('microdata', DEFAULT_VALUE)) should match. Usually we don't enable new features by default for updating users, but in this case it would make sense and make the code easier since we don't have to deal with SQL changes 😄

A thing to consider is to add the microdata type param to the com_content options as well and add a default "Use Global" to the article param. I can see it getting tedious to change the type for each article ;-)

@parthlawate
Copy link
Copy Markdown
Contributor

i kinda agree that i think we dont need to have this as an option at all. atleast not on a per article basis ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does it make sense to have this public? We have the getMicrodata() method to retrieve this property. I don't think extensions should be able to change that property. If we want to allow them changing it, we should provide an API like setMicrodata() which will set the property then.
I would make it private.

The other already existing properties are probably public for BC reasons

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Bakual I agree, but if you take a look at that class, you will see that all the properties are public, don't know why.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm just guessing but I'd say it's to keep backward compatibility with existing code when the scope was added 5 years ago.
However since now introduce a new property, it can be private without issues. We can always make it protected or public later if we find it needed, but we can't change a public property to private later. So I'd better start restrictive 😄

@alexprut
Copy link
Copy Markdown
Author

You set the default value for the microdata param to be enabled and the type to "article" in the XML definitions. But when you retrieve the parameter value you don't set any default values. The result is that microdatas aren't enabled till the user opens and saves the params, and then it magically will be enabled without the user doing something. The same is true for the microdata type. As long as the article isn't saved, the type will be "thing". As soon as the article is saved again, it will take the default "article" type.

@Bakual Yes, that's right, this is because I've not updated the default SQL files used to install the CMS.

A thing to consider is to add the microdata type param to the com_content options as well and add a default "Use Global" to the article param. I can see it getting tedious to change the type for each article ;-)

I completely agree. But I sincerely can't understand why the Article Global Configurations overrides the Article Configurations? (if I disable an option from the Article Options and that same option is enabled in Article Global Configuration, that option is enabled when the article is displayed)
If you setup from Article Global Configurations the Microdata Type it will apply to all articles, even if you change that option in the Article Configurations, at this point to have the possibility to change the Microdata Type for a single article there should be also an option in the Menu Item right?

@alexprut alexprut changed the title Basic JMicrodata implementation in com_content (single article view), also JMicrodata implementation in plugin Content - Vote [#33487] Basic JMicrodata implementation in com_content (single article view), also JMicrodata implementation in plugin Content - Vote Mar 20, 2014
@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 20, 2014

But I sincerely can't understand why the Article Global Configurations overrides the Article Configurations? (if I disable an option from the Article Options and that same option is enabled in Article Global Configuration, that option is enabled when the article is displayed)

That's not how it is supposed to work. The article options should override the global ones. It gets a bit tricky if you take into account menu item params. There a menu item will override article if it's a single menu item pointing to that specific article. Otherwise article will override menu item which overrides component settings. If something doesn't behave like this, it's a bug.

there should be also an option in the Menu Item right?

Would maybe make sense as well. I don't think someone will want to disable microdata for only one menu item, but I could see someone having for example a menu item "blog" and one for "news" where the microdata type would be different.

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 20, 2014

Yes, that's right, this is because I've not updated the default SQL files used to install the CMS.

SQL files would only work for new installations, not for updating users. If you want to enable microdata anyway by default, then you need to do the proper default values when setting the config (what you already did in the XML) and also when retrieving the value (which you didn't do). Those two place need to match in all cases. Even if you set something using SQL files, they still need to match.

Also it looks you the PR is messed up currently :-(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this line supposed to be $microdata = JFactory::getMicrodata($params->get('microdata'))->setType(json_decode($row->attribs['microdata_type'], true));?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@betweenbrain No, the way it is should work.
I saw what you've reported on JTracker, I don't get any warning, anyway, I've made some fixes, can you please test again. thanks

@alexprut
Copy link
Copy Markdown
Author

@Bakual I've added the default values when retrieving a param. Can you please check again?

A thing to consider is to add the microdata type param to the com_content options as well and add a default "Use Global" to the article param. I can see it getting tedious to change the type for each article ;-)

Working on that.

@alexprut
Copy link
Copy Markdown
Author

@Bakual

It doesn't matter much, but can you use $params->get('microdata', 1) instead of $params->get('microdata', true)? So the value is consistent with the actual saved one (since we use 0 and 1 in the XML).

Fixed.

@infazse
Copy link
Copy Markdown
Contributor

infazse commented Mar 24, 2014

Hi Alexandru,
Works fine for single layout articles, good if you can add this to featured articles too. So enabling micro data in the global context does it automatically enable micro data for all articles in default?

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 24, 2014

Enabling/Disabling microdatas in global configuration doesn't seem to have any effect at all. That would probably need a "Use Global" in the component settings as well, and then fall back to global config if "Use Global" is set. Or just remove the global config setting.

After playing around with it a bit more, I think the library implementation is to complicate and inflexible to use compared to just implementing hardcoded microdata. Have a look at PR #3358 to see how easy it actually would be to implement the microdata attributes directly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

otice: Undefined index: microdata_type in /.../plugins/content/vote/vote.php on line 57 if the article opions aren't saved. That's because you don't use JRegistry and no default value here to read the param.

@infazse
Copy link
Copy Markdown
Contributor

infazse commented Mar 25, 2014

Agree with bakul in enabling micro data in global context.

@alexprut
Copy link
Copy Markdown
Author

@infazse @Bakual

Works fine for single layout articles, good if you can add this to featured articles too. So enabling micro data in the global context does it automatically enable micro data for all articles in default?

You're right 👍 , for the parts that are implemented for the moment the Global Configuration only affects the Content - Vote plugin, I'm agree about adding a Use Global in the Article Global Configuration, but: how do I maintain a consistent UI, all the options in that interface are (Yes/No) or (Show/Hide), if I'll add a Use Global option it will result in (Use Global/Yes/No) and break the consistency, or should I use a dropdown list?

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 26, 2014

how do I maintain a consistent UI, all the options in that interface are (Yes/No) or (Show/Hide), if I'll add a Use Global option it will result in (Use Global/Yes/No) and break the consistency, or should I use a dropdown list?

We do that inconsistent currently even within com_content. If you look at the article options, we use lists. If you look at a menu item for a single article, we use radio buttons.
Personally, I would prefer a list.
If you use radios, just make sure you don't use the btn-group-yesno class 😄

@infazse
Copy link
Copy Markdown
Contributor

infazse commented Mar 28, 2014

Yeah a list would do well, The usability in the article options is not very promising, it brings flexibility but in a way its really hard to find an option at times. What about using another field for "Micro Data" just a suggestion :). But com_content options definitely need some re factoring to do in a way web master could easily find an option.

@RCheesley
Copy link
Copy Markdown

A couple of issues/thoughts on this PR:

  1. The date should always be output in ISO format (see http://schema.org/Date and for info on ISO: http://en.wikipedia.org/wiki/ISO_8601) but it doesn't appear to be doing that. You can use a meta tag to make this visible only to crawlers and retain the regular date format if needed. You should also consider adding copyrightYear.
  2. Date published is being used, but in my opinion we should also use date created and date modified as this is relevant to search engines. It should be in a meta tag if that option is set to be disabled in the view, so it's only visible to search engines. A question we've often asked is whether this should be shown at all (even in source) if it's turned off in the view, interested to hear thoughts.
  3. It would be sensible in my opinion to allow people to set the microdata type at a category level, as well as an article level. I wouldn't want to have my end user remember to set it to blog every time they added a blog!
  4. It should be possible to mark up the introtext and main body appropriately using description for the introtext and articleBody for full text
  5. You can nest itemprops - for example in the author you could also use accountablePerson, and perhaps using headline in the article title alongside name.
  6. If an image is used in the article text but there is no image associated via the image upload tabs, this should be (in my opinion) used as the corresponding image for the article
  7. Tags don't seem to be getting marked up. The array needs to be exploded and each one wrapped in probably itemprop="keyword".

I hope that's helpful, will try to help with this if I can!

@betweenbrain
Copy link
Copy Markdown
Contributor

@PAlexcom one of my concerns about this is the amount of PHP code it introduces into the view files. To help mitigate this issue, would it be possible to move the microdata processing into an earlier step in the execution cycle so that it modifies the item properties before this stage? That is $this->item->title would render the same value as $microdata->content($this->escape($this->item->title))->property('name')->display(); if microdata is enable?

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Mar 28, 2014

I wouldn't do it further up the stack. For example the title may also be used in other places like a tooltip where you don't want to have HTML tags around it.

I would rather change the API to be more simpler. For example why do we need that: $microdata->content($this->escape($this->item->title))->property('name')->display();? All it does is wrap a span around the title with the itemprop attribute. We could as well add the attribute to a container which already is there (or add the span manually). I don't need a method to wrap a span around my text. For example we could add it to the <h2> tag instead like this:
`<h2 property('name')->display(); ?>>

escape($this->item->title; ?> ` It's still quite a lengthy command just to echo `itemprop="name"`. We could make it still simpler. Do we need to always explicitely say we want to display it? Why not make it simpler by just using something like `echo $microdata->prop('name');`. Or even more intuitive would imho be `echo $microdata->name`. THAT would be simple 😄

@betweenbrain
Copy link
Copy Markdown
Contributor

Thanks @Bakual, that's a good point about modifying the existing properties.

@AmyStephen's comment at #3358 (comment) got me thinking and I fully agree that we need to prevent "reaching out of the view". Beyond complicating the code in the views, instantiating a class could have performance implementations.

With the very excellent idea of implementing this as a plugin, I'd suggest that we add a microdata property to the item object and do so with a system plugin. This would help keep views cleaner so that we can echo something like $this->item->microdata->type with data that has already been generated to optimize frontend rendering.

To optimize backend performance, we could do this onBeforeSave to update each item, but we'd to also need to fire a one-time event to index items. Maybe that could be done in conjunction to enabling microdata globally, to update all, and them on a category basis to just update that category.

Thoughts?

@alexprut
Copy link
Copy Markdown
Author

alexprut commented Apr 2, 2014

@betweenbrain

To help mitigate this issue, would it be possible to move the microdata processing into an earlier step in the execution cycle so that it modifies the item properties before this stage? That is $this->item->title would render the same value as $microdata->content($this->escape($this->item->title))->property('name')->display(); if microdata is enable?

If the Type (Article, Thing ...) not changed in the meantime, yes :)

@alexprut
Copy link
Copy Markdown
Author

alexprut commented Apr 2, 2014

@Bakual

Do we need to always explicitely say we want to display it? Why not make it simpler by just using something like echo $microdata->prop('name');

Well, because for example: you can set your Property at line 1, and then you set your Fallbacks or Type somewhere else, so until display() isn't called it will not be rendered. It's the same approach when you build a SQL query. What do you think?

@danyj
Copy link
Copy Markdown

danyj commented Apr 9, 2014

Nice work Alex but I have few questions.

So far we have introduced this in com_content which mostly behaves as a blog article.
Now if the user switches the type to recipe , the recommended output is this
http://prntscr.com/38fsk2
instead of
http://prntscr.com/38fsrg

Or see this for a book
http://prntscr.com/38ftae

In Joomla , we are missing half of those tags that we see in recipe/book types,
Does this harm our SEO if they are missing?
Does your lib outputs them on the fly and if yes, this means that we will have more html output
that might actually be displayed unstyled since template devs did not count them in.

Template dev here , hope you understand.

Thank you!

@Chris-Jones-Gill
Copy link
Copy Markdown

Tested with default install of 3.3-dev branch (as of 6pm GMT today) + patch

Functions as expected and described.

One thing I did notice is the author microdata is always the "created by" entry (or their alias) even when an Author is explicitly named in the Publishing tab.
I think that if an author is given then this should override the "created by" for the purpose of the microdata.

@danyj
Any additional properties can be added in a template override, or the author can add them to the post if there are specific extras they need. I added two extra properties to the bottom of a test article...

<footer  class="hidden" >
  <p>Publisher: <span itemprop="publisher">KISS Web Design</span></p>
  <p>Audience: <span itemprop="audience">Joomla Testers for PR#3330</span></p>
</footer>

Although these additions would be visible in the 'articlebody' property when viewing the extracted structured data. This would not be the case with an override.

The output from Google's Structured Data Testing tool looks like this

Item 
type:    https://schema.org/blogposting
property:   
inlanguage: en-GB
url:    Microdata test
name:   Microdata test
author: Item 1
genre:  Uncategorised
datepublished:  2014-04-10T18:22:01+0000
articlebody:    This article should have some default microdata Publisher: KISS Web Design Audience: Joomla Testers for PR#3330
publisher:  KISS Web Design
audience:   Joomla Testers for PR#3330

Item 1
type:    https://schema.org/person
property:   
 name:  Chris Jones-Gill

A template override would be the best way to add any properties that are not included.

Either way though there is no impact on SEO for not using some of the available properties, just look at how many are listed for the Blog schema http://schema.org/Blog and nobody uses all of them.

Perhaps an example override file (with lots of comments about what, why and how) could be produced for template devs to use as a reference - and then added to the documentation. Not sure if I'll have time to do this myself, hence leaving the note here ;-)

@brianteeman
Copy link
Copy Markdown
Contributor

One thing I did notice is the author microdata is always the "created by" entry (or their alias) even when an Author is explicitly named in the Publishing tab.
I think that if an author is given then this should override the "created by" for the purpose of the microdata.

I would definitely consider that a bug

@alexprut
Copy link
Copy Markdown
Author

@danyj

Now if the user switches the type to recipe , the recommended output is this
http://prntscr.com/38fsk2
instead of
http://prntscr.com/38fsrg

Well as you know by default the com_content was designed to be used and output Articles/Blog, but the end user can use it to write Recipes or everything else (there are 558 different types). The only valid information that we know and are in common is http://prntscr.com/38fsrg

In Joomla , we are missing half of those tags that we see in recipe/book types,

We don't and can't know what type of semantics/context the author uses in the content (article body), so that type of information needs to be added by the author (planned for the future: to add semantics from the content editor).

Does this harm our SEO if they are missing?

As I know it should not harm the site SEO, but I know for sure that if the semantics are used in the wrong way for sure will harm the website, the library takes care to check and display valid semantics.

we will have more html output
that might actually be displayed unstyled since template devs did not count them in.

Yes and No, in the current com_content view override yes, some <span> tags will be added, instead if you create your own override you can use the library to output only <meta> tags.

Template dev here , hope you understand.

We need your help :) The current implementation is a bit complex, I've created a new post in order to get some feedback and simplify the current implementation https://groups.google.com/forum/#!topic/joomla-dev-general/s0SR7hyfW_I

@danyj
Copy link
Copy Markdown

danyj commented Apr 11, 2014

Thnx Alex, I saw your post on JGD,
see this
http://webmasters.stackexchange.com/questions/60472/will-wrong-or-incomplete-microdata-harm-my-seo

According to the answer , Google might cut you off completely if microdata is "manipulated"
still does not give me an answer if the missing tags or wrong microdata is "seen" as manipulation.

I am finishing our framework and yes we do use our own overrides. Our framework works on 2.5.x and 3.x
so I am kinda being forced here to add custom microdata settings for 2.5 , but as you know the yaml from schema.org is huge and there are so many options.

still waiting to see what you make out of it so that I can fit it in 2.5.x or just display the microdata on 3.x.
But please do something about that syntax if you can. It is to long.

@alexprut
Copy link
Copy Markdown
Author

@dongilbert @KISS-Web-Design

I would definitely consider that a bug

I would not consider it a JMicrodata library bug

@brianteeman
Copy link
Copy Markdown
Contributor

If we are outputting the wrong author then its a bug.

@AmyStephen
Copy link
Copy Markdown
Contributor

@PAlexcom - agree with @brianteeman - here is an example of how to determine $author.

@danyj
Copy link
Copy Markdown

danyj commented Apr 15, 2014

@PAlexcom see mys post here please https://groups.google.com/forum/#!topic/joomla-dev-general/s0SR7hyfW_I

@ghost
Copy link
Copy Markdown

ghost commented Apr 18, 2014

I am a little confused, I tested PR's #3252 and #3330 (toggle button in the global configuration for Microdata), but I also see PR's #3358 and #3359 with hardcoded Microdata.

On which branch should I test #3358 and #3359?

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Jun 4, 2014

I'm closing this PR as it is against an outdated branch.
Alex is accepted to do a second GSoC project this summer which will result in a new PR to include this functionality.
Thanks all for the comments and testing. It's much appreciated!

@Bakual Bakual closed this Jun 4, 2014
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.