replace all .srcElement with .target

[andiHaven]

.srcElement is non-standard and is not supported in Firefox, so this causes some bugs in Firefox. .target is supported everywhere (IE >= 9 and all other browsers).

[selsamman]

At present BINDSter has backwards compatibility to XP-only browsers (e.g. IE 8). In all the cases you patched we were testing to see if target existed. The only exception was the return value which was to correct an IE only problem. Can you let me know the nature of the bug you found with Firefox.

[andiHaven]

I didn't realize Bindster supported IE8; I asked around and was told that it didn't, so that's why I just removed srcElement. We can certainly add more conditions to test target vs srcElement, rather than my just getting rid of srcElement all together. But target should be the default since it's part of the standard, and srcElement should be used only when target fails.
The actual problem I found which required that fix was that checkboxes on the HIT admin site using b:onclick weren't getting checked in Firefox when I clicked on them. In other browsers, b:onclick returned true for a checkbox, but because of Firefox not supporting srcElement, the return value of b:onclick incorrectly ended up as false.
The main problem was in these do_return lines where it went straight to srcElement without testing if target existed:
var do_return ="return (ev && ev.srcElement && ev.srcElement.tagName && ev.srcElement.tagName == 'INPUT' ? true : " + do_return + ");";

[selsamman]

Yes that is what the docs say on bindster.com So that is odd about the b:onclick problem in admin. In Bindster there is a test html file multivalue.html. It has a checkbox: <input b:bind="val6" type="checkbox" b:truevalue="true" b:falsevalue="false" b:onclick="val5 = val6"><br /> and then a span that binds to val5 which is set in the onlclick <span b:bind="val5"></span>(<span b:bind="typeof(val5)"></span>) Oddly the span shows the updated value of val5 when you click the checkbox. Is this to do with a specific version of Firefox (e.g. that MM uses) vs the latest. I know that was an issue before regarding the hoisting of functions embedded in an if/else.

…

On Tue, Dec 20, 2016 at 3:41 AM, andiHaven ***@***.***> wrote: I didn't realize Bindster supported IE8; I asked around and was told that it didn't, so that's why I just removed srcElement. We can certainly add more conditions to test target vs srcElement, rather than my just getting rid of srcElement all together. But target should be the default since it's part of the standard, and srcElement should be used only when target fails. The actual problem I found which required that fix was that checkboxes on the HIT admin site using b:onclick weren't getting checked in Firefox when I clicked on them. In other browsers, b:onclick returned true for a checkbox, but because of Firefox not supporting srcElement, the return value of b:onclick incorrectly ended up as false. The main problem was in these do_return lines where it went straight to srcElement without testing if target existed: var do_return ="return (ev && ev.srcElement && ev.srcElement.tagName && ev.srcElement.tagName == 'INPUT' ? true : " + do_return + ");"; — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#10 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABYATl495n1csFawEMf4GJGNqARh6tpuks5rJ0B3gaJpZM4LPbyR> .

[andiHaven]

bindster.com says "Compatible back to IE6"... is that true? and if so, you intend to keep it that way?

In the example above, I think it's expected that val5 would show the updated value of the checkbox.
The checkbox checked property is first updated, then the onclick event handler is run. (and then if the onclick event handler returns false, the checkbox is reverted to its original state.)
It's a strange sequence of events, but I think people just have to be aware of that when creating event handlers.

For the original issue, it has nothing to do with the version of Firefox. Firefox just doesn't support srcElement, and those do_return statements rely on srcElement without testing for target.

[andiHaven]

I just updated the PR to keep the tests for srcElement, but this should still fix the Firefox issue.

[selsamman]

Still having trouble reproducing the problem. I changed the handler to a function call which returns false but the state of the checkbox does not revert. Can you submit a separate PR that modifies the test to reproduce the problem. Otherwise there is no way to confirm the PR that fixes the problem. I pushed the change to multivalue.html that I was using to attempt to reproduce this. ....Sam

…

On Tue, Dec 20, 2016 at 6:24 PM, andiHaven ***@***.***> wrote: I just updated the PR to keep the tests for srcElement, but this should still fix the Firefox issue. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#10 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABYATgm4HjDpPSJtsDimE3gL3-ba10T2ks5rKA9IgaJpZM4LPbyR> .

[andiHaven]

Hi Sam,
The UW team was using checkboxes in perhaps a non-standard way for Bindster; the checkboxes were not bound to a value, but the onclick handler was used to run a function when the checkboxes were clicked, and the checkboxes were expected to toggle as normal.
Try replicating the issue with code like this:
<input type="checkbox" b:onclick="node" /> (or other similar onclick event that does nothing, just for the purpose of having an onclick handler there.)
Andi

[selsamman]

OK in a normal b:onclick handler you do have the privilege of returning true/false to affect default handling. Except that this used to screw up IE and hence the code that references srcElement. So I think the answer is to return true in your event handlers if you want default handling. I think this behaviour can be up for discussion but this is the way it works for now. ...Sam

…

On Wed, Dec 21, 2016 at 1:35 PM, andiHaven ***@***.***> wrote: Hi Sam, The UW team was using checkboxes in perhaps a non-standard way for Bindster; the checkboxes were not bound to a value, but the onclick handler was used to run a function when the checkboxes were clicked, and the checkboxes were expected to toggle as normal. Try replicating the issue with code like this: (or other similar onclick event that does nothing, just for the purpose of having an onclick handler there.) Andi — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#10 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABYATjy5zBr3MUWqiqLqfJOQnXkemY-dks5rKSszgaJpZM4LPbyR> .

[andiHaven]

I'm confused on what you mean here. I think my updated changes will benefit Firefox without having a negative effect on IE, because I just added a check for ev.target in addition to the check for ev.srcElement, so the logic for IE should not have changed. Let's talk in person next week if the test case or anything isn't clear.

[andiHaven]

Just added the test case in /test/multivalue.html. Please take a look at that checkbox in both Firefox and Chrome, before and after including the other changes in this PR.