X-Cart forums

[cflsystems]

Just to remind everyone about the <font> tag used in XC
http://forum.x-cart.com/showpost.php?p=371623&postcount=210

Now it seems the XC5 code also uses deprecated code. At least one place I found it

/Includes/functions.php

PHP Code:

/**
* Uploads SQL patch into the database. If $connection is not defined, uses
* mysql_query($sql) syntax, otherwise mysql_query($sql, $connection);
* If $ignoreErrors is true, it will display all SQL errors and proceed.
*/
function query_upload($filename, $connection = null, $ignoreErrors = false, $is_restore = false)
{
    $fp = @fopen($filename, 'rb');
    if (!$fp) {
        echo '<font color="red">[Failed to open $filename]</font></pre>' . "\n";
        return false;
    } 


PHP Code:

} elseif ($table_name != "") {
                echo '<font color="green">[OK]</font><br />' . "\n";

            } elseif (!($counter % 20)) {
                echo '.';
            } 


PHP Code:

function query_upload_error($myerr, $ignoreErrors)
{
    if (empty($myerr)) {
        echo "\n";
        echo '<font color="green">[OK]</font>' . "\n";

    } elseif ($ignoreErrors) {
        echo '<font color="blue">[NOTE: ' . $myerr . ']</font>' . "\n";

    } else {
        echo '<font color="red">[FAILED: ' . $myerr . ']</font>' . "\n";
    }
} 


etc. Other files <font> tag is used:
skins/default/en/common/range_validator.tpl
skins/admin/en/shipping/charges.tpl
skins/admin/en/modules/CDev/TinyMCE/js/tinymce/plugins/bbcode/plugin.min.js
skins/admin/en/modules/CDev/TinyMCE/js/tinymce/plugins/paste/plugin.min.js

So this is XC5 which is supposed to be new modern up to date shopping cart.
Not to mention it is still using HTML4 and CSS2 (CSS3 partially as part of bootstrap I guess).
Now why is that?

[totaltec]

Steve, come on man. You know as well as I do that these echoes are really not supposed to see the light of day. I don't know what this code is you are looking at, but just by reading it you can see that it is not for a user interface, more likely some kind of status message. All of it is located in the admin or common sections, not customer facing.

Some of the <font> tags that I can find are from
1. Bootstrap
2. TinyMCE
3. Doctrine
4. FontAwesome

The ones that actually are core XC5 code and not some plugin or lib, are all error messages and warning messages. Most of them are dealing with the installation process, before CSS files and the like even properly exist. I found 40 occurrences across 17 files.

I personally am much more worried about the dreaded <table> tag. I found 205 matches in 117 files. I hate tables. They should all be destroyed!

But saying XC5 uses deperecated code is a little harsh, and a little wrong. I admit, yes there is the now deprecated font tag, proof right there in the pudding. A deprecated tag that is still supported by every browser in existence. I doubt that browsers will every really drop support for the font tag. What they need to do, is drop support for those pesky tables!

[cflsystems]

This is just an example. There is more to it. This shows pattern in coding. It doesn't matter if it shows for admin only or for all or if browsers support it. There are still a lot of IE6/7 users - should XC5 be coded for them as well?

It is the principle. If you have a member in your team who is still coding the way of the 80s and no one says anything how do you know all the code passes validation?

This applies to tables as well - the use of tables is not something to throw away but they have to be used when appropriate. XC uses tables exclusively - still - and as I see it is no matter how many times this is said - someone on the XC team continues using them.

This is why I am posting this. You may think it is nothing just because it is in one place in admin - but it shows consistency with not following standards. QT makes me validate my code to certain standards before they accept it and at the same time their code does not validate... but you think this is ok. (this is not a complain about them making me validate the code - I think this is good practice)

This may be a nitpicking but it has to stop before we end up with XC5 code from the 80s. I stand behind my nitpicking. It is 2014 and HTML5 is the standard which I do not see in XC5 instead there is code deprecated a decade ago.

[totaltec]

I feel you Steve. Agreed on the code validation and not following standards. We should all do that. I've riled a bit when my code had to pass validation, but I could see blatant examples in the core that did not.

I'll concede the point. Still think the title is too inflammatory, in general I disagree, XC5 uses some darn fine code that is very current by my standards.

[tony_sologubov]

Hi Steve!

Thanks for raising the question. You are absolutely right that X-Cart's code must follow the standards as you are saying and we want to polish the code to this point. We are not doing it right now, because of other tasks priorities.

This is a time in X-Cart's life when we must rapidly grow and extend functionality. We sacrifice a lot of developer-focused tasks for it, because nobody will care about developer niceties as long as the software does not meet general requirements in terms of functionality.

So, my answer will be:
a) please report such problems in our bug-tracker, because they should be fixed
b) I cannot promise that we will fix such problems shortly, because a lot of other work is more important for X-Cart 5 development at the moment

[cflsystems]

Thank you Tony, hopefully XC team will fix these. Not bugs but still...

[cflsystems]

Ok now this is another piece and exactly what should NOT be happening. This is not nitpicking.

So I am working on a module and I need to load iframe in a popup. XC already includes colorbox and uses it on product details page so this is perfect, exactly what I need as colorbox can load iframe content with very little effort.

So everything coded and testing - what a surprise - my content doesn't load in the popup. I tried everything possible, started digging through the XC code for similar uses, etc - nothing. It shows inline content but not iframe. I spent like an hour and a half trying to make the damn thing work without any luck.

As a last resort I decided to check the actual colorbox.js file in XC as there were no errors showing or anything, just empty content. What a surprise - the colorbox js file supplied with stock XC does not support iframe. Are you kidding me? I have been using this for the last 2-3 years at least with no issues.

So the colorbox version supplied with stock XC5 is 1.3.15 released on October 27, 2010. yes that's right 4 years ago. I would understand if this is like current version or almost current but latest colorbox is 1.5.14 released on Sept 9, 2014, so a month ago. I am not saying QT has to jump immediately on the latest when it is released but at least stay close to latest. 69 versions of colorbox were released after the one used in XC5.

Just tell me I am not right....

Tony

This is a time in X-Cart's life when we must rapidly grow and extend functionality. We sacrifice a lot of developer-focused tasks for it, because nobody will care about developer niceties as long as the software does not meet general requirements in terms of functionality.

Things like this do stop the growth and effect functionality. I did sacrifice a lot of developer-focused time just because XC5 uses sooo old piece of code when newer one is available. There is no excuse for that. I wrongly assumed stock 3rd party software used in XC5 is up to date or at least close to what's current.

Now I have a ready module which partially does not work. I can't load another copy of colorbox on top of the existing one as this will create a conflict and none of them will work. I can't replace the one that comes with XC5 because with next upgrade my copy will be overwritten...

So what do I do?

Well lets decorated Gallery class to remove stock colorbox.js and css and add newer version through my module. This works but for something stock I shouldn't have to go through this right...

[tony_sologubov]

Hi Steve!

You are right: colorbox lib should be updated. The reason why we did not do that earlier is everything was working properly, but I am not denying that for developers' convenience we should have done this earlier.

I am sorry that you wasted so much time digging into it.

Please report the problem to bt.x-cart.com and the team will include this task into the queue.

Tony.

[cflsystems]

Thank you Tony. Posted in the BT. I guess it is partially my fault since I assumed X5 uses newer version of the colorbox and did not bother to check. For XC4 I always replace it but there is easy - just upload new file and done....