checkboxes error

This bug occurs for views containing multiple instances of a checkboxes field using either the "Editable" or "Click to Edit" formatter.

The problem is the checkboxes forms do not give their elements page-wide unique IDs. (Note that each checkboxes field is contained in its own form.) Normally, Drupal would give each form's elements an incrementing ID, e.g., "edit-field-myfield-value-foo-1-wrapper" and "edit-field-myfield-value-foo-2-wrapper". This does not occur with editablefields in this scenario; instead you would get "edit-field-myfield-value-foo-wrapper" twice. This confuses the browser.

These duplicated IDs are easy to confirm by using Firebug to inspect the form elements.

I believe the reason these elements do not get incrementing IDs is that editablefields uses Ajax to do a separate HTTP request to get each form (instead of the usual single request per page.) form_clean_id() is the Drupal API call responsible for taking in an ID and returning a unique version of it. It is called during the rendering process for a form. Internally, form_clean_id() uses a static array $seen_ids. Since each form is a separate HTTP request, the static $seen_ids is reset between each call. Therefore, form_clean_id() doesn't know that a particular ID has already been used on the current page, and returns a duplicate ID.

Could not figure out a clean way to fix this. I ended up hacking core (includes/form.inc) to check for my particular field of interest, and add a distinguishing string to the ID for each. If you'd like I could post my core modifications.

Does this make sense? Anyone have an idea of how to fix it in the module?

I found the fix:

It was a JS error, which did occur in firefox and sometimes in chrome.

Changing just the id field in JS to be unique is enough to fix that two or more events are triggered.

Best Wishes,

Fabian (LionsAd)

Nice fix, but a random number won't ensure uniqueness--the random number could repeat for two fields in the same view.

Hi,

Nice that you like the fix!

Very unlinkely, but you are right :-):

New patch attached, which uses a static incrementing index and as JS has no true parallelism, it is safe to use.

Best Wishes,

Fabian (LionsAd)

Works great. Modified to also fix radio buttons.

Hi
Could you roll a patch against the latest CVS (the 6.2 branch)
There is a fix in there for single check boxes, which I think is likely to be of interest too...
see: http://drupal.org/node/353110

Cheers

Mark.

subscribing. i have the same issue(s) on the 1.2 branch

I believe your patch 'hid' the problem - or - rather - your patch is required if there is more that one open edit at the same time - or something like that

The real problem - I think - was that the html being feed back from the module had (for some strange reason) the editable version of the field in it - which is exactly not what you want. For all other 'click to edits' then once edited, and focus is lost, they return to the normal state....

Turns out the code used to generate the html was using the wrong rendering mechanism - whcih was the route of all evil. So - I've changed that

Hopefully now it works a dream.... let me know if your mileage varies.

If I get some +ve feedback, I'll make a non-dev release.

Cheers

Mark.

Mark, I checked out the latest code from DRUPAL-6--2 and had two problems:
1. Using "Click to Edit", you could only change a field's value once.
2. Using "Editable (Ajax)", the problem still exists where if you click a radio in say the third radio buttons field of the view, the first radio buttons field is also modified. (This issue was originally for checkboxes, but I think it's really checkboxes and radio buttons.)

I also noticed that the "edit" links above each "Click to Edit" field from 6.x-1.2 are gone. Maybe this is by design? It's hard for a user to know the field is editable without these.

Hi,

I disagree: I like to edit several rows at a time without the thing being closed - it is much more comfortable and I see what I had edited before.

Here is another patch, which also fixes the last checkbox and single checkbox problem in the Javascript.

You could call it a workaround - however the first thing is a strangeness in how browser have checkboxes with "onchange" event setup by id and not by element.

The second is also a workaround as for me the issue just comes with having "1" as the checkbox key, with on off and other words it works.

However you could also call it a bug in the serializer and such the workaround is valid.

Anyway, perhaps this is useful to some 1.x users.

Best Wishes,

Fabian

Thanks Fabian,

For the 2.x branch, I have tried not to change too much about Jan van Diepen's intent (as I understood it) - so I think he decided to make the 'click to edit' just that - and leave the themeing to distinguish between click-to-edit things and 'normal' things...

For all non-check boxes and radios - the editable field dissapears once done with. Hence I believe this was a bug with check boxes and radios -which is - I believe - now fixed (there was a race condition on blur and on-change events which caused some strangeness for me - but I've fixed that now)

I have to say, most requests were for 'click to edit' to behave this way - rather than to stay open.... you could - of course- have everything editable to start with anyway.

threexk : I can't re-produce your problems at all on the latest CVS code?

Mark,

That is a very valid point and I thank you for your insightful reply.

But as far as I have understand the 1.x branch is behaving now the way that fields stay open - is that correct?

If that was the case, how about you commit my workaround to 1.x with a note saying that it is a Work-Around until you can find a proper fix for 1.x, too.

As the patch is quite tiny I don't think there are side-effects.

I tested it with 1.x branch and it does apply without problems.

Best Wishes,

Fabian (LionsAd)

markfoodyburton: Maybe our browsers are behaving differently? I am using Firefox 3.5 (Windows, also tried Firefox 3.0 in Linux and the problems I listed happen there too.) As I said before I am using the DRUPAL-6--2 editablefields branch. I did cvs update, cleared the cache, and retested before making this comment--problems still occurred.

Attached are exports of a bare-minimum content type and view you could use to reproduce the problem. Creating two nodes with different values for the radio buttons field is sufficient. Go to the view and select a new value for the second node's field. The value of the first node's field will be changed (undesirably).

I also tried checkboxes and same problems with them.

#12 ... Good plan :-)
So - I've committed this on the DRUPAL-6--1 branch.

Please let me know if this works for you...

Thanks again

Cheers

Mark.

#13 - Ahh - I see - the problem is with radio's on ajax edit.... - both click to edit, and 'html' mode work...

I've added the 'belt and braces approach' - so that now the id field is made unique (as per the patch above), and committed this (on the 6--2 branch)

Also, I have taken the liberty to add back something for the [edit] thing.
The reason, in the end, I think this is better is that its only in the Php code that we can determine language - so, if people want something saying [edit] - in the right language - then leaving it to the theme is too late.

However, the theme can very easily 'hide' the element.

I've added some animation to it, to remove it when your not interested in it, so HOPEFULLY 9/10 cats will be happy.

I thought about some sort of config option, but thats a pain to organise in a sensible way...

Hope everybody enjoys :-)

Cheers

Mark.

6--2 works great now in all three modes, thanks! (Haven't tried 6--1 yet.) I like the compromise solution where the edit buttons appear when you mouse-over the element. Seems a little strange how they flash on and off a couple times if you're holding the cursor still, though--don't know whether that is intentional.

To be crazy I tried it in IE8, and the behavior there is different. Once you click on a different radio for a field, the new value is not applied. It is only applied once you click somewhere else. In other words, changing a value requires two clicks. This is different from the Firefox one-click behavior. Maybe a separate issue here?

I get the flicky behaviour too - not sure, I guess this is some sort of Firefox/Jquery interaction thing... I guess we could work round it - but :-)

As for IE... I dont have IE, so I can't debug that too well.... I'd welcome some fixes there, but good to know it's reasonably workable

If you think it's solid enough for a release, let me know

Cheers

mark.

Tested the 6--1 branch and it works for both formatters and both checkboxes and radios in Firefox. Seems good enough for a release to me (6--1 and 6--2). There's the IE8 problem, but checkboxes/radios are now much better than they were, which was mostly broken.

Suppose I'll add a new issue for the IE8 problem, unless it's covered by one of the existing IE issues... Will try to fix it, but I have very little JavaScript experience.

Binding to the click event rather than the change event for updating the node fixes the IE8 problem. Would this break anything else? (Other editablefields field types besides checkboxes/radios?)

Some comments on this page about the problem:
http://bytes.com/topic/javascript/answers/538606-internet-explorer-oncha...

I tested these patches in both IE8 and Firefox 3.5 with all formatters. They are created from latest code in CVS.

IE7 also works with the previous comment's patches.

Hi, I've tried a slightly different fix to the one you suggest (the one later on the same page you pointed at) -- and I've made it, I hope, so it only effects checkboxes and radios --- for me, it makes no difference ! (Thats good :-) ) (It's checked in)

Can you check for IE?

BTW, while I was checking this out, I noticed something very annoying, at least for me - it seems that Firefox randomly decides to 'check' some boxes (presumably trying to remember what I did last time on this form- or some such)....

The effect is that if you have a bunch of checkboxes, and you press one, then reload the page, both that one, and another one seem tobe checked. But if you reload the page again, you get what you expected... and if you look at the source code, both of them are correct - so I think this is some sort of Firefox strangeness, and I'm not going to chase it more --- but if you notice the same, can you shout ;-)

IE8 doesn't work with the latest 6.x-2.x code. When you click on a radio or checkbox, it changes but nothing happens (i.e., no throbber and the node isn't updated.)

Drat and double drat
Well - I've made it use the 'click' event as you initally suggested. Seems to work still for firefox... let me know for IExploder.
I've tried to limit the damage to just radio's and checkboxes.... Hopefully we're safe...

Yuk :-(

Cheers

Mark.

Sorry that I don't know JavaScript well enough to come up with a more elegant fix.

Both checkboxes and radios tested successfully in IE 8 and Fx 3.5. I did not test IE 7. IE 7 worked the same as IE 8 before, though. I probably didn't test all three formatters with every combination of browser/field type.

Hope you will consider backporting this fix to the 6--1 branch (as long as 6--2 remains dev, at least.)

Given the work we've put into the 2.x branch, I think it's time 2.x became the supported release version. There seem to be some benefits to it now....
So - I'm inclined to release a 6--2.0 -- what say you?

Cheers

Mark.

Sounds good. 6--2 now works just as well as 6--1 for my purposes. Thanks for your work on this issue.

Done :-)
this is now fixed in 2.0, and I'll mark it closed

Thanks for the help

Cheers

Mark.