Suitecrm: EditView and QuickCreate tab order ignored - tabindex setting not set

[dhayner]

Issue

I've set tab order numbers for all contact fields in the QuickCreate layout for Contacts. Yet when the page is rendered, all the tabindex's show -1 or no value at all.

Expected Behavior

The set tab order should be followed, so the user can easily move through forms in the intended field sequence.

Actual Behavior

Original/nonsensical tab order is used, or basically none. Tabbing through fields skips by and ignores most of them. Repair/Rebuild after changing the tab ordering in Studio has no effect.

Possible Fix

Unknown where these changes are not being carried from Studio into the actual rendered site. Server cache issue?

Steps to Reproduce - can be done on SuiteCRM Demo too.

1. In Studio> Contacts> Layouts> QuickCreate, adjust the tab ordering so First Name is one, Last Name is two.
2. Go into an Account. In the Contacts subpanel, click Create.
3. Click into into the first name field. Enter text.
4. Hit the tab key on the keyboard, expecting perhaps to move into the Last Name field as the tab order is set. You move across into the Account/Relate field instead.

Context

This causes large amounts of user frustration for those who are entering Contacts all day, as well as in other areas where tab order is ignored. Accounts>EditView is also affected, at least.

Your Environment

• SuiteCRM Version used: 7.9.0 and 7.9.1
• Browser name and version (e.g. Chrome Version 58.0.3029.110 (64-bit)):
• Environment name and version (MySQL 5.7.11, PHP 7.0.3):
• Operating System and version (Windows 10):

[fosullivan-usam]

Same issue for us. Any help appreciated.

[geconly]

Line 137 is the culprit \include\SugarFields\Fields\Base\SugarFieldBase.php.

The code was changed from
function getSmartyView($parentFieldArray, $vardef, $displayParams, $tabindex = -1, $view) [which works]

The new code adds a different criterion and this messes up the tabindex order:
function getSmartyView($parentFieldArray, $vardef, $displayParams, $tabindex, $view){
// set $tabindex = -1 by default
if(!is_numeric($tabindex)) {
$tabindex = -1;
}
[which does not work]
the !is_numeric causes the issue, I believe.

Go back to the old function parameters.

[pgorod]

@geconly thanks for the diagnosis, it's helpful.

I am wondering, though, that if that change was made it must have been for a good reason. Perhaps some other nonsensical value was passed in to the function from somewhere.

So maybe it needs fixing if isnumeric doesn't cut it, but maybe it needs fixing in a different way, not just going back to how it was. Can you see what type is being passed in when is_numeric($tabindex) returns false?

I'm guessing it's a string like "3" that just needs a simple conversion to number...

[geconly]

I asked myself the question, "what element would I NOT want to be part of the natural tabindex flow?" I couldn't think of one other than modals which are handled correctly . I believe it was just a programming oversight.

On Jul 4, 2017, at 1:15 AM, pgorod [e-mail] wrote:

@geconly thanks for the diagnosis, it's helpful.

I am wondering, though, that if that change was made it must have been for a good reason. Perhaps some other nonsensical value was passed in to the function from somewhere.

So maybe it needs fixing if isnumeric doesn't cut it, but maybe it needs fixing in a different way, not just going back to how it was. Can you see what type is being passed in when is_numeric($tabindex) returns false?

I'm guessing it's a string like "3" that just needs a simple conversion to number...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[pgorod]

Well, it doesn't look like an oversight because it was done on purpose. The question you could be asking yourself instead is "what other value was being passed to GetSmartyView that was causing a problem somewhere, and made a programmer go into that function and change it?" :-)

@gymad can you remember why you made this change? It seems to be raising problems... Maybe is_numeric is not the better test to use. Thanks.

[geconly]

What I meant by "oversight" is that it wasn't properly tested and the ramifications for the choice weren't completely considered.

On Jul 4, 2017, at 9:01 AM, pgorod [e-mail] wrote:

Well, it doesn't look like an oversight because it was done on purpose. The question you could be asking yourself instead is "what other value was being passed to GetSmartyView that was causing a problem somewhere, and made a programmer go into that function and change it?" :-)

@gymad can you remember why you made this change? It seems to be raising problems... Maybe is_numeric is not the better test to use. Thanks.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[sergio91pt]

That change seems to be about fixing the "Incorrect usage of default arguments".
If it broke the tabindex feature, the problem probably lies with the caller and not is_numeric. This function returns true to int's and numeric strings.

[geconly]

The function should be getting the correct number of parameters anyhow.

On Jul 4, 2017, at 12:32 PM, Sérgio Faria [e-mail] wrote:

That change seems to be about fixing the "Incorrect usage of default arguments".
If it broke the tabindex feature, the problem probably lies with the caller and not is_numeric. This function returns true to int's and numeric strings.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[geconly]

Okay, then just remove that:
function getSmartyView($parentFieldArray, $vardef, $displayParams, $tabindex, $view)

[javitoron]

Well, I finally found where this fields, and tabindex values of course, were generated.

The include/EditView/EditView.tpl has been overwritten by the themes/suiteP/include/EditView/EditView.tpl.

This SmartyTemplate calls tab_panel_content.tpl in the same folder... Well, as I'm using SuitePImproved this wasn't true, suitePImproved/include/EditView/EditView.tpl was loading suiteP/include/EditView/tab_panel_content.tpl so that had to be fixed.

Anyway, in the tab_panel_content.tpl file you find tabindex=$tabindex 4 times, that $tabindexare replaced by:

1. $subfields.tabindex
2. $colData.field.tabindex
3. $colData.field.tabindex
4. $colData.field.tabindex

And this is working everywhere for me.

This change is not upgrade-safe because I didn't get loaded that tpl from the custom/themes... I'll come back later to fix that

[gymad]

@pgorod nice catch, It was my fault when I refactored this part of the code to set the default value of the argument.
Thanks

[fosullivan-usam]

Thanks javitoron! I made the changes and it works correctly again.