The 'crossed wires between tab sessions' bug - Still there in 3.3, argh!

[alanms]

I don't like bringing bad news but 3.3 still has some instances of the infuriating and dangerous bug where form/page submissions sometimes get posted to the wrong place if more than one entity of the same type is open in separate tabs (see http://forum.civicrm.org/index.php?topic=12643.0  and http://forum.civicrm.org/index.php?topic=16328.0 and http://forum.civicrm.org/index.php/topic,16000.0.html ...)

So far it's been observed for notes (easy replication: 1. Open several contacts in seperate tabs from the 'recent items' block 2. Add notes for each of them using 'Add -> Add note' 3. Look surprised when each note is saved to the last contact opened) and also several administration items in 'Option lists' (e.g. 'settings' for custom fields - here it has the less dangerous but more bizarre behaviour of switching 'edit' forms to 'add' forms on submission causing data validation failures). I'm sure there's many more; I think I spotted it with a particular use of activities recently (might have been case activities), but these are harder to replicate.

This is causing us huge problems. We've got a senior member of staff leaving very shortly who needs to add personal notes to all his several hundred contacts for continuity. This bug is making an already time-consuming task into a major headache: I've got him opening strictly one extra tab in addition to the search results page, which does work and saves continually rerunning searches and delicate mashing of the back button, but it's not ideal. When doing a task like this you should be focusing on the task in hand, not on tiptoeing around the limitations of the software.

Just a little more transparency would make a big difference to fixing a bug with as many instances as this one. All we'd need to get stuck in splatting this bug once and for all would be:

- A note explaining what causes it, with a commented example patch which we can adapt to new bug sightings. I couldn't find anything on the issue tracker (maybe my poor search skills) nor in the various forum discussions
- A list of everywhere the bug has been fixed, with links to the appropriate page on the demo, file locations of files patched, and patches.
- A list of all 'sightings' of the bug not yet fixed, with a link to the appropriate page on the demo.

With this, it'd take someone like me less than 20 minutes to splat a new bug sighting like this and submit a patch, and it'd be easy to pre-emptively hunt down and splat fresh instances of this bug without having to wait for users to be bitten by them.

Without this, for a community member like me to fix a damaging instance of this bug would require many hours re-inventing diagnostic work that's already been done.

[alanms]

If anyone could offer even just one example of a patch that fixed another instance of this bug that'd be a big help.

update: Lobo just gave me the url for this on the issue tracker via IRC: thanks! http://issues.civicrm.org/jira/browse/CRM-5612

[Donald Lobo]

Hey Alan:

This issue is being tracked and fixed here. It has a number of sub-issues (and will get more as we find a few more places where its not fixed)

You can also find all the code changes are linked to the issue (click on the fisheye link). In general we prefer to track issues / commentary / status / file fixes via the issue tracker. Maintaining seperate wiki pages seems like a lot more work. Most "sightings" of bugs come in via the forum and via IRC

Here are the places i know which are still not fixed:

1. All profile forms (primarily because we support standalone forms and do not use qfKey)

2. All "simple" forms. A simple form in CiviCRM is one which is invoked via a CiviCRM "page" (check: http://civicrm.org/achitecture/ for descriptions). Someone needs to go in and either "split" the code into different actions and/or write code to work around it.

Looking forward to your note patch. Check the changes to CRM-5250 for one way on how to fix this patch and/or ping us on IRC

If you'd like to create a wiki page to list all the places where its still not fixed, please do so

lobo

[alanms]

Thanks for the reply (and IRC advice!).

For now I'm adding un-fixed 'sightings' as sub-tasks of the 'Multiple tab - Session issues' on Jira as linked above - seems the most sensible place for it, keep everything in one place. Really hoping the fix to these bugs is generalisable... I'll put any useful information for others in the comments on the main issue in Jira.

Quick link to how this was fixed for CRM-5250: https://fisheye2.atlassian.com/viewrep/CiviCRM/trunk/CRM/Custom/Form/Option.php?r1=26310&r2=26883

[alanms]

Wow, finally had time to look at this again and just found something rather relevant.

I knew it'd be down to something relying on session data for form saving data but I didn't realise it'd be as nakedly simple as this:

CRM/Core/Controller.php line 473 - called from CRM_Core_Form::get()

Code: [Select]

function get( $name ) {
        return self::$_session->get( $name, $this->_scope );
   }

Huh. Well that'd explain it.

I wonder if an easy and generalisable fix could be to change this line to use parameters by default and session data only as a fallback - which in theory, could make fixing each instance of the bug as simple as defining an appropriate parameter on page load? Any thoughts on any likely collateral consequences from this? I'm experimenting but won't be able to test extensively.

[Donald Lobo]

this wont work in a few cases:

1. when the form validation fails, since the GET params are not resent (but stored in the session)

2. In the case of a multi-form process (import/mailing etc), where only the first form sees the GET params

3. In general we use the session as a store to also store internal variables which are not GET/POST parameters

lobo

[xavier]

1. when the form validation fails, since the GET params are not resent (but stored in the session)

Can't it be solved by putting it in the POST instead of the session ? (and check the post variable first, and fallback on the session variable=

2. In the case of a multi-form process (import/mailing etc), where only the first form sees the GET params

Can't it be put in the POST (or in the subsequent GETs)

3. In general we use the session as a store to also store internal variables which are not GET/POST parameters

Would it be expensive (eg. code to change in a lost of places) that civi always use an index for each of the session variables (mostly for the wizards), and pass that index in the get of the wizard?

eg. every time you start a wizard, you generate a unique id (like for the search results),  and saves the variables the wizard uses in this "namespace".

For what I understood, some "normal" forms are still using the same stack as the wizards (ie. are seen as a one step wizard). Not quite sure I fully understood it, nor what's the benefit of it.

X+

[Donald Lobo]

I dont think we are too keen to go in and change the workings of the PEAR packages especially since we are planning to transition away from them in the next few releases

lobo

[alanms]

...no progress on this?

On my side, I've got a small and growing pile of crude fixes for the very worst symptoms, but without some serious knowledge and understanding of the session system (and maybe also quickforms keys) I can't make these close to patch-worthy.

For example, I've fixed the frightening bug where, if you open a tab to delete group/tag "A" and open a tab to edit group/tag "B", when you "Okay" the "Are you sure you want to delete group/tag A" page, it actually casually deletes group/tag B instead.

This is through simply saving the ID as a hidden field then checking for that in pre process before using session data - barely a 20 minute task after figuring out where to look. However, for some reason I can't begin to understand with my minimal quickforms knowledge, on both the groups and tags pages, every form tab after the first now does nothing at all on submitting except redirect to the list page. It doesn't even seem to get as far as the form/something.php preprocess function.

For another example, I've got a similar simple crude fix for the potentially equally serious bug where if you add multiple notes in multiple tabs, they all get saved to the last person opened. This time, the problem that remains is with the page the user is redirected to - even though the data is now saved to the correct contact, they're taken to the page of the latest contact to be opened. This is minor compared to the contact relations nightmare of important notes ending up attached to the wrong person, but it looks unprofessional and shoddy, and it's deeper than just overriding a url saved to session data.

So is anyone who actually understands quickforms and/or the pear session package actually working on this?

Also, why is this family of bugs listed in Jira as 'Closed' and 'Trivial'? Surely it's rather serious if a respected CRM system has a bug that can cause staff to unknowingly delete important groups or tags, or if it can result in staff getting in contact with the wrong people based on incorrectly stored information? Especially since, when the problem first occurs, there's very little clue that anything is wrong.

[Donald Lobo]

The bugs are not closed, there are still a couple that u've filed that are still open:

Check: http://issues.civicrm.org/jira/browse/CRM-5612

Ping us on IRC and we can help u out with the QF issues. Would be great to get patches to fix some of these "leftover" session issues.

lobo

[Chris Burgess]

Some older discussion on this topic -

• http://forum.civicrm.org/index.php?topic=2807.0
• http://forum.civicrm.org/index.php?topic=10310.0
• http://issues.civicrm.org/jira/browse/CRM-2876