? Pending

User tests: Successful: Unsuccessful:

avatar photodude
photodude
18 Jan 2016

We have the JMicrodata library, lets actually use it.

Testing
1. Add patch
https://docs.joomla.org/Testing_Joomla!_patches
2. Add a popular articles module to display
3. Check that microdata values are applied using a microdata validation tool
https://developers.google.com/structured-data/testing-tool/

avatar photodude photodude - open - 18 Jan 2016
avatar photodude photodude - change - 18 Jan 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 18 Jan 2016
Labels Added: ?
avatar photodude photodude - change - 18 Jan 2016
Title
Add JMicrodata library for schema.org microdata
Add JMicrodata for schema.org microdata
avatar photodude photodude - change - 18 Jan 2016
Title
Add JMicrodata for schema.org microdata
Add JMicrodata to popular articles module for schema.org microdata
avatar sandstorm871 sandstorm871 - test_item - 3 Feb 2016 - Tested successfully
avatar sandstorm871
sandstorm871 - comment - 3 Feb 2016

I have tested this item :white_check_mark: successfully on e5481b1

Added "Most Read" Module after patch & confirm that Microdata is now shown in the validation tool.
J3.5 Beta2


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8934.

avatar anibalsanchez anibalsanchez - test_item - 5 Feb 2016 - Tested successfully
avatar anibalsanchez
anibalsanchez - comment - 5 Feb 2016

I have tested this item :white_check_mark: successfully on e5481b1

Test OK


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8934.

avatar joomla-cms-bot
joomla-cms-bot - comment - 29 Feb 2016

This PR has received new commits.

CC: @anibalsanchez, @sandstorm871


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8934.

avatar photodude
photodude - comment - 29 Feb 2016

@anibalsanchez, @sandstorm871 No actual change, just Rebased the branch to staging...

avatar anibalsanchez anibalsanchez - test_item - 2 Mar 2016 - Tested successfully
avatar anibalsanchez
anibalsanchez - comment - 2 Mar 2016

I have tested this item :white_check_mark: successfully on 96f41d7

Test OK


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8934.

avatar brianteeman brianteeman - change - 9 Mar 2016
Status Pending Needs Review
avatar brianteeman
brianteeman - comment - 9 Mar 2016

Set to needs review for same reason as #8933


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8934.

avatar bertmert
bertmert - comment - 9 Mar 2016

This PR makes no sense.
Property name is not supported by Google.
Article without an Image less than 695px width is invalid.
Article without a publisher is invalid.
and so on.

Nearly all structured data markups in Joomla are (Google) invalid meanwhile because the required combinations of properties are nearly never given.

Thus, only unnecessary HTML/markup.

https://developers.google.com/structured-data/rich-snippets/articles#article_markup_properties

#8933 (comment)

avatar photodude
photodude - comment - 10 Mar 2016

@bertmert This PR Just follows the same format already used in the hard-coded Joomla microdata for the latest articles module. The difference being the use of JMicrodata library rather than hard-coded structured data.

This PR is part of the Larger discussion in consideration for whether to use Hard-coded microdata for the structured markup or to use the JMicrodata library.

IMHO, Discussions about IF the existing "structured data markups in Joomla" are necessary or correct, are completely separate from the discussion that this and the other PR bring up. As such, your concerns would be more valuable discussed in the mailing group

avatar bertmert
bertmert - comment - 10 Mar 2016

I understand but still think it's useless and a bad idea to change now already unsufficient/wrong Microdatas to JMicrodata.

If you want to remove/"deactivate" Microdata (in overrides) you have to find and remoive after PRs like this every single JMicrodata-Snippet to avoid PHP errors if you want to avoid loading of unneeded class JMicrodata in every file.

With hardcoded ones you just have to remove itemscope attributes to avoid conflicts with e.g. application/ld+json.

(BTW: It's more performant to leave them hardcoded. But just said.)

avatar photodude
photodude - comment - 10 Mar 2016

@bertmert I'm not agreeing or disagreeing, I just feel there is a better place for that discussion which would have more overall impact.

As for performance, I would agree that hard coded microdata would have better performance on a "non-cached" site. In the same way that a fully hard coded HTML file will be faster than a CMS that is dynamically serving content and queries a database. But the logic to that argument against using a dynamic library, for an encoding that can be dynamic, just because there is a performance difference in a non-cached setup; falls a bit flat for me as I believe a CMS should be capable of being dynamic, overridable, and simple to the end user.

I haven't run the numbers, but I doubt the performance impact of using the JMicrodata library in a non-cached site would be noticeable or significant vs Hard-coded markup. But I'm willing to accept that I could be wrong. Testing that and proving that there is a noticeable or significant difference for using the JMicrodata library vs hard-coded markup (in non-cached and cached) could make a difference in the overall discussion (or it would prove that it's a non-issue). Unfortunately, I don't have the time to do the testing right now.

avatar bertmert
bertmert - comment - 10 Mar 2016

Performance was just a little addition at the end of my post.

avatar photodude
photodude - comment - 19 Mar 2016

@bertmert I took a look at your comment and the details in the supplied link

Property name is not supported by Google.
https://developers.google.com/structured-data/rich-snippets/articles#article_markup_properties

From the link I can see how you came to the conclusion that name is not a supported property for Google. It's easy to assume that name is not supported when it's not in the example. But that's misreading the example. You must look at the full schema.org/Article type to see what's supported. name is a parent property for articles from the schema.org/Thing type. This is why the example links to the full schema.org/Article type so you can read the full specification and not just the supplied example. You can see that name is a fully supported property for an article type by just running a test against Google's microdata validation tool

avatar brianteeman brianteeman - change - 22 Mar 2016
Category Modules
avatar wilsonge
wilsonge - comment - 7 May 2016

Myself, Chris, Roland and Viktor have just discussed this and I think our decision is to stick with hardcoding this for the same reasons we chose not to use it in the first place. I'm going to close this PR and the other module ones for this reason

avatar wilsonge wilsonge - change - 7 May 2016
Status Needs Review Closed
Closed_Date 0000-00-00 00:00:00 2016-05-07 11:05:19
Closed_By wilsonge
avatar wilsonge wilsonge - close - 7 May 2016

Add a Comment

Login with GitHub to post a comment