ZC158 Meta Tag constructions on Home page

[torvista]

I think the structure of the ternary operators is incorrect and possibly also the logic. There are changes in php8 for ternary operators which may be a factor.

zencart/includes/modules/meta_tags.php

Lines 97 to 101 in ee3ee15

	case ($this_is_home_page):
	define('META_TAG_TITLE', (defined('HOME_PAGE_TITLE') && HOME_PAGE_TITLE != '' ? HOME_PAGE_TITLE : (defined('NAVBAR_TITLE') ? NAVBAR_TITLE . PRIMARY_SECTION : '') . TITLE . TAGLINE));
	define('META_TAG_DESCRIPTION', (defined('HOME_PAGE_META_DESCRIPTION') && HOME_PAGE_META_DESCRIPTION != '') ? HOME_PAGE_META_DESCRIPTION : TITLE . PRIMARY_SECTION . (defined('NAVBAR_TITLE') ? NAVBAR_TITLE : '' ) . SECONDARY_SECTION . KEYWORDS);
	define('META_TAG_KEYWORDS', (defined('HOME_PAGE_META_KEYWORDS') && HOME_PAGE_META_KEYWORDS != '') ? HOME_PAGE_META_KEYWORDS : KEYWORDS . METATAGS_DIVIDER . (defined('NAVBAR_TITLE') ? NAVBAR_TITLE : '' ) );
	break;

To test, I stuck text in front of each:

case ($this_is_home_page):
define('META_TAG_TITLE', 'META_TAG_TITLE:  ' . (defined('HOME_PAGE_TITLE') && HOME_PAGE_TITLE != '' ? HOME_PAGE_TITLE : (defined('NAVBAR_TITLE') ? NAVBAR_TITLE . PRIMARY_SECTION : '') . TITLE . TAGLINE));
define('META_TAG_DESCRIPTION', 'META_TAG_DESCRIPTION:  '. (defined('HOME_PAGE_META_DESCRIPTION') && HOME_PAGE_META_DESCRIPTION != '') ? HOME_PAGE_META_DESCRIPTION : TITLE . PRIMARY_SECTION . (defined('NAVBAR_TITLE') ? NAVBAR_TITLE : '' ) . SECONDARY_SECTION . KEYWORDS);
define('META_TAG_KEYWORDS', 'META_TAG_KEYWORDS:  ' . (defined('HOME_PAGE_META_KEYWORDS') && HOME_PAGE_META_KEYWORDS != '') ? HOME_PAGE_META_KEYWORDS : KEYWORDS . METATAGS_DIVIDER . (defined('NAVBAR_TITLE') ? NAVBAR_TITLE : '' ) );
break;

Note Description and Keywords are now broken (they work without the prefixed text).

While the parenthesis can be fixed, it’s not obvious to me how things should be.

So after some clarification in words I’d like people to review and comment if it looks ok as is.

FYI and mine, some background on definitions:
PRIMARY_SECTION is a colon separator
SECONDARY_SECTION is a hyphen separator
TERTIARY_SECTION is a comma and space
METATAGS_DIVIDER is a space. I thought this should be a comma and space.
TITLE is defined (default “Zen Cart!”)
TAGLINE is defined on the fly:
if SITE_TAGLINE is not empty (default “The Art of E-commerce”) then TAGLINE is TERTIARY_SECTION . SITE_TAGLINE
if SITE_TAGLINE is empty then TAGLINE is empty
KEYWORDS is a sanitized CUSTOM_KEYWORDS (default ”ecommerce, open source, shop, online shopping, store”)

'META_TAG_TITLE'
HOME_PAGE_TITLE is always defined but default is empty
If HOME_PAGE_TITLE is not empty, use HOME_PAGE_TITLE. TITLE . TAGLINE

If HOME_PAGE_TITLE is empty:
If NAVBAR_TITLE is defined, use NAVBAR_TITLE . PRIMARY_SECTION . TITLE . TAGLINE
If NAVBAR_TITLE is not defined, use TITLE . TAGLINE

'META_TAG_DESCRIPTION'
HOME_PAGE_META_DESCRIPTION is defined but may be empty.
If HOME_PAGE_META_DESCRIPTION is not empty, use HOME_PAGE_META_DESCRIPTION
If HOME_PAGE_META_DESCRIPTION is empty, use TITLE . PRIMARY_SECTION . (use NAVBAR_TITLE if defined) . SECONDARY_SECTION . KEYWORDS
If no NAVBAR_TITLE, then the PRIMARY_SECTION and SECONDARY_SECTION are concatenated.
Why are KEYWORDS on the end of a description?

'META_TAG_KEYWORDS'
'HOME_PAGE_META_KEYWORDS' is always defined, may be empty.
If HOME_PAGE_META_KEYWORDS is not empty, use HOME_PAGE_META_KEYWORDS
If HOME_PAGE_META_KEYWORDS is empty, use KEYWORDS . METATAGS_DIVIDER . (use 'NAVBAR_TITLE' if defined)

[proseLA]

sorry @torvista, you have a lot going on here and i want to understand it.

the title of this issue contains 'possible errors'. what are the possible errors? i think the code is difficult to read, but i'm not sure what you think is the error.

you then go on to say that there are changes in php8's handling of ternary operators. if so, can you point out an example in these lines of code that come up with a different definition in php 7.4 v php 8?

you then run a test. but your test breaks everything. so i'm not sure what your test shows.

what parenthesis are broken that you want to fix them?

this is what i did. before the break on line 101, i added the following code:

      echo "---META_TAG_TITLE----->" . META_TAG_TITLE . "<---------<br>";
      echo "---HOME_PAGE_TITLE----->" . HOME_PAGE_TITLE . "<---------<br>";
      echo "----META_TAG_DESCRIPTION---->" . META_TAG_DESCRIPTION . "<---------<br>";
      echo "--HOME_PAGE_META_DESCRIPTION------>" . HOME_PAGE_META_DESCRIPTION . "<---------<br>";
      echo "-META_TAG_KEYWORDS------->" . META_TAG_KEYWORDS . "<---------<br>";
      echo "---HOME_PAGE_META_KEYWORDS----->" . HOME_PAGE_META_KEYWORDS . "<---------<br>";
      echo "--PRIMARY_SECTION------>" . PRIMARY_SECTION . "<---------<br>";
      echo "--SECONDARY_SECTION------>" . SECONDARY_SECTION . "<---------<br>";
      die(__FILE__ . ':' . __LINE__);

i then went to the home page, and this is what i saw:

---META_TAG_TITLE----->Zen Cart!, The Art of E-commerce<---------
---HOME_PAGE_TITLE-----><---------
----META_TAG_DESCRIPTION---->Zen Cart! : - Hardware Software DVD Movies Gift Certificates Big Linked Test Examples Free Call Stuff Test 10% by Attrib Test 10% A Top Level Cat Sale Percentage Sale Deduction Sale New Price Big Unlinked New v1.2 Music Documents Mixed Product Types ecommerce, open source, shop, online shopping, store<---------
--HOME_PAGE_META_DESCRIPTION------><---------
-META_TAG_KEYWORDS------->Hardware Software DVD Movies Gift Certificates Big Linked Test Examples Free Call Stuff Test 10% by Attrib Test 10% A Top Level Cat Sale Percentage Sale Deduction Sale New Price Big Unlinked New v1.2 Music Documents Mixed Product Types ecommerce, open source, shop, online shopping, store <---------
---HOME_PAGE_META_KEYWORDS-----><---------
--PRIMARY_SECTION------> : <---------
--SECONDARY_SECTION------> - <---------
/var/www/zcdev/includes/modules/meta_tags.php:110

these definitions did not change if ran it under php 8.1 or php 7.4.

the only real problem i saw (which we are talking about meta stuff in the header) is this part of the code:

(defined('NAVBAR_TITLE') ? NAVBAR_TITLE : '' ) . SECONDARY_SECTION .

i would potentially change that to:

(defined('NAVBAR_TITLE') ? NAVBAR_TITLE . SECONDARY_SECTION : '' )  .

but i'm not really sure its necessary.

if i am missing something, please let me know. but i do not see how the code is wrong; nor do i see how 7.4 and 8.1 would come up with different results.

but i do think the code is confusing and some of the constants are not well named, but i'm not inclined to change any of it.

[torvista]

Firstly, thanks for taking the time to plough through all that. Since I do all this before I keel over post-midnight it's surprising any of it makes sense.

I put a bit of extra text before/concatenated with the rest of each definition, and so expected to see that text prefixing each item.
That didn't happen.
Title: ok, text was prefixed.
Description and Keywords: not ok, the texts disappeared.
After two days in the subconscious I can now see that due to the placement of the brackets in the nested ternary operators, the prefixed text actually becomes part of the true/false test in both cases.
So, the structure is not logically wrong, just hard to see the process, depending on the state of one's mind.
I did alter the brackets so it produced results as expected, and can submit that, but really nested ternary operators should be broken down to if else to make it all easier to understand.

So, a simple question, why are keywords tagged into the description?

[proseLA]

Firstly, thanks for taking the time to plough through all that. Since I do all this before I keel over post-midnight it's surprising any of it makes sense.

you are welcome.

I put a bit of extra text before/concatenated with the rest of each definition, and so expected to see that text prefixing each item. That didn't happen. Title: ok, text was prefixed. Description and Keywords: not ok, the texts disappeared. After two days in the subconscious I can now see that due to the placement of the brackets in the nested ternary operators, the prefixed text actually becomes part of the true/false test in both cases. So, the structure is not logically wrong, just hard to see the process, depending on the state of one's mind. I did alter the brackets so it produced results as expected, and can submit that, but really nested ternary operators should be broken down to if else to make it all easier to understand.

i like you and am willing to do some debugging for you; but again, your title contains "...possible errors..." and i am just not seeing that.

as far as coding style and nested ternary operators "should" be broken down.... well i am not the owner of the repo, nor can i merge anything.... so as to why something is the way it is, one would need to look at the history and see. it is an open source project, so anyone can contribute, and if the owners want to merge it, then they can merge it. coding style is not a conversation that i'm overly interested in; but i agree that the code is hard to read. i do not agree that it contains errors (other than the small point i made in my first post on this thread).

So, a simple question, why are keywords tagged into the description?

so now we are into the black arts of SEO, of which i am far from an expert. so again, one would need to look at the history and see if it makes sense in today's environment. it seems that the only times these KEYWORDS are used is in the meta data. so i'm not sure what the question is. if you do not want keywords in the meta description, define them as an empty string. clearly they were put here to help with SEO and search engines being able to properly index one's site.

[torvista]

Ignoring seo-lore and it's ir/relevance, I take Description at face value to mean complete sentences describing the product but Keywords to be single words. So having single words tagged onto a description is incorrect.