admin, Move Product: infobox duplicated/--> PHP Warning: Undefined array key 10 in ceon_uri_mapping_javascript.php on line 306.

[torvista]

https://github.com/torvista/Zen-Cart_CEON-URI-Mapping/blob/95547b87d81535485cb13c3726e2e0a4d21c2c52/files/ADMIN_FOLDER/includes/ceon_uri_mapping_javascript.php#L306

Looking at this: https://github.com/torvista/Zen-Cart_CEON-URI-Mapping/blob/95547b87d81535485cb13c3726e2e0a4d21c2c52/files/ADMIN_FOLDER/includes/ceon_uri_mapping_javascript.php#L298-L318

This method addURIMappingFieldsToProductMoveFieldsArray((int)$_GET['pID']); adds the URI fields onto the global $content variable which already contains all the elements for this infobox created in admin\includes\modules\move_product.php

So when the for loop is started, $contents has 10 elements 1 key [form] 9 keys [0-8] which are added in move_product.php 10 key [9] the URI radio buttons:

So the error mentioned occurs when the for tries to read key [10] which does not exist.

That's "fixed" by $n = count($contents)-1

But subsequently, the whole infobox content is inserted after the original. The javascript which does that, and that of Copy (which works correctly), leaves me dizzy, so some help required....

[neekfenwick]

My goodness, that code needs to be taken out in the woods and shot in the face. It's a PHP file (so the resulting javascript is inline in the browser making it harder to debug), overrides window.onload (breaking anything else that wants to use onload), writes javascript that uses global PHP variables... my head hurt after a few minutes trying to understand it :) Might try another look if I find time.

[torvista]

my head hurt after a few minutes trying to understand it

That makes me feel better.

This sort of thing is why I argue there should be this functionality (not this code) in the core, to not waste the time of the few, keeping orphan add-ons alive.

[neekfenwick]

This sort of thing is why I argue there should be this functionality (not this code) in the core, to not waste the time of the few, keeping orphan add-ons alive.

I would tend to agree. Think I saw some discussion a few years ago about how there is so much technical debt in core that it's best to keep this kind of this in an addon, but I'm not sure what the argument was. I've never used it but I keep seeing a "Products URL" field on the Edit Product page, just above the shipping details section. I think it has been there since many versions ago, I started with 1.3.x, but for some ancient reason I used Ceon URI Mapping addon instead. Does the core functionality suck, or something? I'll have to take a closer look some time. Without rewrite rules, like Ceon uses, I'm not sure how it would work, maybe it still needs index.php in the URL or something i.e. not actually SEO friendly.

[torvista]

The Products Url field is used for the "More Info" link, so I use it to point to the original manufacturers product page....which on thinking about for the first time in x years, is probably a bad thing (providing a link away from your site!).

I also started with 1.38 and Ceon appeared to be the best, and in the years I kept this alive after Conors passing, I have to say it is generally so well-written that the tweaks to keep it going with php versions have been minimal.

Still, I argue that people who want an ecommerce site want friendly urls and so it should be core functionality if you want people to use your offering. Pure marketing. Having the best product with a declining user base is pointless.

[neekfenwick]

The Products Url field is used for the "More Info" link, so I use it to point to the original manufacturers product page....which on thinking about for the first time in x years, is probably a bad thing (providing a link away from your site!).

Ah that rings a bell. Thanks.

I also started with 1.38 and Ceon appeared to be the best, and in the years I kept this alive after Conors passing, I have to say it is generally so well-written that the tweaks to keep it going with php versions have been minimal.

Yeah, while I have issues with some of the code formatting it generally seems well designed. Every single page on our site, from login, account stuff, products, RMA/returns, and custom pages I've added all have a Ceon powered SEO friendly URL.

Still, I argue that people who want an ecommerce site want friendly urls and so it should be core functionality if you want people to use your offering. Pure marketing. Having the best product with a declining user base is pointless.

I think the argument I heard years ago is that zencart is platform agnostic, and doesn't want to specify a specific web server tech, although I'd say that LAMP/WAMP is probably by far the most common and any server worth its salt supports rewriting in a way that the Ceon stuff requires. Anyway, probably not going to solve this debate on this thread, eh? :)

[torvista]

ok, well then I guess we have to keep fixing this for the moment! Unless Ceon Support has some input....if not to fix it, at least to green-light an approach to rework this particular thing.

[neekfenwick]

I think I've found the bug. It's caused in part by the weird way the author likes to duplicate large amounts of code that really could be refactored. I'm too lazy to dig into the code history, the way you seem to have forked the code means there is no git blame history to easily look at.

I have a more thorough write-up but here's a simple one:

Compare the code for 'copy_product' where a variable $contents_start is used to only add onto $ceonUriMappingCopyProduct those elements from $content starting after the core fields. The index of the latest element at the time is captured (7 or 8, whatever), then the addURIMappingFIeldsToProductCopyFieldsArray() function is called to append any number of elements to $content, then $content is iterated from indec $contents_start onwards.

https://github.com/torvista/Zen-Cart_CEON-URI-Mapping/blob/95547b87d81535485cb13c3726e2e0a4d21c2c52/files/ADMIN_FOLDER/includes/ceon_uri_mapping_javascript.php#L247-L261

Then look at the code for 'move_product', where no such $contents_start stuff is done, the for loop that uses $i to take HTML from $contents starts at 0.

https://github.com/torvista/Zen-Cart_CEON-URI-Mapping/blob/95547b87d81535485cb13c3726e2e0a4d21c2c52/files/ADMIN_FOLDER/includes/ceon_uri_mapping_javascript.php#L296-L308

Please see PR #12

[neekfenwick]

This bug would be obviated by clearing out $contents before appending to it and then just using its entire contents without having to worry about $contents_start.

Or by not using $contents at all, since by this time its usage in core ZC code is completely finished and we are in a handler for ADMIN_FOOTER_END where javascript is being used to dynamically modify the DOM after page rendering. But, I guess there is sense in using a consistent approach to UI building and the way $content is used is consistent with core zencart.

[torvista]

Your PR fixes the duplication, but not the error for undefined array key 10. The contents array does have 10 elements but the first key is "form", followed by keys 0-9, so there is indeed no key 10.

While just subtracting -1 from the count for ($i = $contents_start, $n = count($contents)-1; $i < $n; $i++) { fixes this, it strikes me that things could/should be simplified here, as element "2" contains the drop-down for the destination categories, which is my case is 6300+ lines!

But, I still can't see the wood for the trees in this section of code, as I tend to get round to looking at these things when I should be asleep....

[neekfenwick]

Your PR fixes the duplication, but not the error for undefined array key 10. The contents array does have 10 elements but the first key is "form", followed by keys 0-9, so there is indeed no key 10.

Hmm, well, I do not get this error message, so I suspect some other addon you're using is introducing some weirdness. If you're still up, can you give screenshot or more details on this error message? It's a PHP error, not a Javascript one in the browser, right?

It sounds like clearing out the global $contents variable before the ceon stuff tries to add to it would solve this problem. I tried this yesterday but the way it's accessing $GLOBALS['contents'] gave me trouble getting the right variable into scope, I need to brush up my PHP skills.

[neekfenwick]

I have pushed a second commit to my PR, which removes the ridiculous loop counter stuff, clears out the global $content before ceon writes into it, and then just puts the content from Ceon into the javascript string, and thus the DOM node. Hopefully this will fix your array index problem, but I couldn't reproduce it and don't see where it was coming from.

Would be happy to look again if you give more details :)

[torvista]

That fixes the array problem. Apart from having the local site in strict mode, I don't have the my usual strict mode in the file itself. This thread fixed in https://github.com/torvista/Zen-Cart_CEON-URI-Mapping/pull/12