Token Input: Fix Firefox blur problems

[marekhrabe]

Due to some browser event inconsistencies, in Firefox, there is a strange bug that makes TokenField steal focus back even after you selected another input and interacted with it.

Steps to reproduce

• Use Firefox
• Open up New post
• Select tag input
• Insert some tag
• Focus Post title input and try to enter "test"

WordPress.com	This Branch

On WordPress, you should be only able to enter the first letter before tag input steals back the focus. This PR fixes that.

---

It is fixed by moving onBlur listener to <input> (instead of div) and adding preventDefault as a response to mouseDown on suggestion elements.

I tested this in Chrome, Safari and Firefox. I have not tested IE, I hope someone can.

• Fixes Components: Refactor TokenField focus and blur handling #144
• Fixes Editor: lose focus on editor during auto-save #2682
• Fixes Editor: Tags TokenField doesn't blur correctly in Firefox #4313

Test live: https://calypso.live/?branch=fix/token-input-blur

[marekhrabe]

There were three tests failing, I fixed one of them by editing it to test exactly what it describes and not to test side-effects of that action cc524a7. I removed a source of those side effects in this PR — clicking a suggestion no longer triggers blur, because it preventsDefault on mouseDown event. That allowed me to really simplify the problem here.

There are currently two failing tests left (they are both targeted specifically for Firefox), but honestly, I have no idea what to do with them. They test that if you can click anywhere in the field, the component will be activated. That seems to work in browsers (including Firefox) but it fails in tests.

It might be related to the fact, that I removed some onClick as by my other changes, it was rendered useless. There is no need to listen for onClick anymore because we listen for onFocus on the parent element and it catches clicks correctly. I think it is safe to remove those two tests, but I'd like some other opinion on that.

I saw that @gziolo wrote those tests and @nylen fixed some issues regarding the Firefox blur previously. Can any of you (or anyone else) help me test the functionality and give me your suggestion on removing tests? Thanks

[gziolo]

I only refactored those tests to work with Enzyme. @nylen can confirm if it is okey to remove them.

BTW, you have 5 failing tests on Circle CI at the moment:

Your build ran 3351 tests with 5 failures

all of them complain about:

TypeError: this._clearBlurTimeout is not a function

[marekhrabe]

TypeError: this._clearBlurTimeout is not a function

Oh, thanks for noticing this one. I have made everything synchronous, so there is no need for the timeout anymore. However, I left one call there which caused tests to fail. It should be fine now. Only those 2 tests fail

[nylen]

Thanks for working on this. I would love to see this messy code removed, but there are some bugs with the current implementation.

In Chrome, if you type part of a tag, then click the post title, the tags input steals the focus. After that, if you click the tags input again, the current token is added (it seems the code gets confused about whether the field is active):

Same behavior in Safari, but everything seems to work well in Firefox.

I'd recommend getting an IE virtual machine from http://modern.ie/

This change also needs to be tested on mobile browsers. I ran through it on Android Chrome and iOS Safari (v8.2), everything seems OK on both of these. Do you have any others handy that you can test with?

I agree that the tests are tightly coupled to the implementation at the moment. Let's get everything working well then circle back around to the tests.

[nylen]

Since the new _onBlur code is very simple we can remove this debug statement...

[gziolo]

You have changed behavior of _onBlur handler. Those 2 failing tests simulate blur event, so that probably has some connection. I don't have time to investigate that further this week, I hope you can figure it out yourself ;)

[marekhrabe]

@nylen Thanks a lot for finding the bug that slipped under my radar. The problem was that _addCurrentToken (which onBlur is calling), was checking state.isActive and forcing focus if it is true. I added a reset for isActive, just before calling _addCurrentToken. It seems to be working well now.

[nylen]

Found another bug... since the onBlur handler was moved from the div.token-field to the input we can trigger a blur by clicking on the div. I've added background: #fcc to the input to illustrate the issue.

[nylen]

Do we need onTouchStart here as well?

[marekhrabe]

I've tested this on the iPhone and mouseDown seems to do the job.

[nylen]

Took a shot at fixing the bug I noted above, and cleaned up the tests. Flipping to Needs Review.

[marekhrabe]

Thanks for going ahead and fixing the problem & tests :)

I just tested this in Chrome, Firefox, macOS Safari, iOS Safari & IE11. It seems to work consistently everywhere now. I've been mainly testing to blur token input by different methods (clicking outside, clicking inside, click on suggestion, Tab) and I haven't found focus stealing anymore, which was the original problem this PR fixes. After a code review, I think this is ready to be merged.

[nylen]

Nice work again, and thanks for taking another look!