Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'hasfocus' binding causes focus loss #352

Merged
merged 3 commits into from
Jul 26, 2012
Merged

Conversation

mbest
Copy link
Member

@mbest mbest commented Jun 6, 2012

http://jsfiddle.net/mbest/NZ6sY/

Compare two input elements, A with hasfocus and B without.

If A has focus and you switch to another tab/window and back, it not longer has focus.
If B has focus and you switch to another tab/window and back, it still has focus.

The 'hasfocus' binding should not cause an element to lose focus when you switch to another window and back.

@mbest
Copy link
Member Author

mbest commented May 11, 2012

When a browser will goes out of focus, the element in focus is sent a blur event. The hasfocus binding then updates the attached observable, which triggers the binding's update function, which then calls blur. Now the browser won't re-focus the element when you switch back to the browser window.

A fix would be for the update function to not call blur in this situation.

@mbest
Copy link
Member Author

mbest commented May 11, 2012

See #355 for a version of the update function that doesn't call blur at all. Is there really a good use-case for needing to blur a field?

@SteveSanderson
Copy link
Contributor

Here's a case that makes use of the blurring functionality: http://jsfiddle.net/bh3r5/

It's a bit weird, I know. If it comes down to a tradeoff between losing this functionality, and losing the active element when switching tabs, I'm not sure which I'd prefer. But we could solve the tab-switching problem by not calling blur if it was caused by hasfocus itself triggering an update to the underlying observable?

@mbest
Copy link
Member Author

mbest commented Jun 6, 2012

Here's a version that retains the blur functionality, but uses activeElement to determine if the element is "in focus".

This change makes the binding represent whether it's active rather than focused and doesn't currently pass the specs (but the specs are a bit faulty).

…the focus state. This avoids the binding causing the current element to "blur" when switching to another window.
@mbest
Copy link
Member Author

mbest commented Jun 6, 2012

I made a minor change to specs so that they pass with the hasfocus change. It turns out that triggering focusin/focusout doesn't actually change the focus of an element. Thus the spec, although working before, was faulty.

@ghost ghost assigned mbest Jun 6, 2012
@mbest mbest mentioned this pull request Jun 6, 2012
@SteveSanderson
Copy link
Contributor

Excellent - thanks! This looks like an ideal solution.

One final tweak - since we still list Firefox 2 as a supported target for KO, and since Firefox 2 doesn't support activeElement, I've tweaked the code to fall back on the event name for determining focus for such old browsers. Also I renamed writeValue just to make the code easier to read.

Ready to merge if you're happy with this change (or I'll merge it if you confirm).

@mbest
Copy link
Member Author

mbest commented Jul 25, 2012

since we still list Firefox 2 as a supported target for KO

Why are we still supporting Firefox 2? Even jQuery doesn't support it: "IE 6.0+, FF 3.6+, Safari 5.0+, Opera, Chrome".

Anyway, the changes are good; I just wish we didn't have to bother supporting it. :-(

…changing tabs/windows by setting a flag that prevents it from calling 'blur()' on the element when it loses focus.
@mbest
Copy link
Member Author

mbest commented Jul 26, 2012

I made a change so this problem will be fixed in all browsers. See http://jsfiddle.net/mbest/QzBjH/1/ for a demonstration that uses this technique instead of activeElement. I have kept the activeElement logic in the binding, though, because it also prevents the observable from being set to false when changing windows.

@SteveSanderson SteveSanderson merged commit afdbfc2 into master Jul 26, 2012
@SteveSanderson
Copy link
Contributor

Nice one, thanks.

The only reason we're still supporting Firefox 2 is that it's listed as a supported browser on the project homepage. At some point we might want to change this and even consider dropping support for the oldest IE versions, since lots of other JavaScript libraries these days have abandoned IE < 8. (jQuery 2.0 even requires IE 9.)

Fortunately there are hardly any cases where supporting Firefox 2 (as opposed to Firefox 3.5+) actually affects our code - this is probably the only instance so far.

@omnisk
Copy link

omnisk commented Apr 12, 2013

After recent my project migration to jquery 1.9.* and ko 2.2.1 I faced problem with hasfocus binding. on IE8 and IE9 ActiveElement does not work...

isFocused = (ownerDoc.activeElement === element);
it returns "Unspecified error", google cannot help me much, however there is one related topic:
jquery-archive/jquery-mobile#2064

btw, when I tried to copy/paste and override this handler with some minor change, I noticed that ko.dependencyDetection.ignore and ko.expressionRewriting.writeValueToProperty became unspecified in production enviroment (I use some bundle packaging and code minification). Maybe it was my fault, because these changes worked for me in localhost enviroment. but still it's fishy...

@mbest
Copy link
Member Author

mbest commented Apr 12, 2013

@omnisk - I think that problem is described here: #703

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants