Clicking outside an empty text element makes it unfocusable

[bfintal]

This is better explained by following these steps:

1. put your cursor on the end of a paragraph
2. press enter to create an empty paragraph
3. click just a bit above or below the empty paragraph (outside to make the caret go away, but not on another element)
4. you can't click on the empty paragraph anymore to put the caret back inside.

May be related to #116

[anthonyjb]

Empty paragraphs are removed once they lose focus, this is the intended behaviour.

[bfintal]

Yup I know. But if you don't click on another element, but just enough to lose the caret, they don't go away. Perhaps that should be the bug :)

[anthonyjb]

Ah I see, it's almost like it's removed from the virtual DOM but not the actual DOM. Will take a look.

[bfintal]

Thanks!

[anthonyjb]

OK it was a bit more complex than I at first thought because we've been relying on not blurring an element through native 'blur' events for a while now. Adding back native blur support broke a number of tests and it took a while to fix them.

Also the scope of this fix is a little more far reaching than I'd hoped so if you spot anything odd directly related to it please let me know.

[bfintal]

Thanks for the fix. Found something unwanted. Now if you have a blank text element then click on the heading tool, the element now disappears. Prior to the fix the element won't disappear :)

I think it's because the element lost focus because the toolbox was clicked.

[anthonyjb]

Yeah I think we're going to have to back this out and think about it some more¸had a feeling it might have bigger knock on effects.

[bfintal]

I agree, I think instead the fix should be to bring force the caret back on click if it's not there.

[bfintal]

I think this issue got closed again by accident :)

[anthonyjb]

Yeah the revert automatically closed it because of the message

[bfintal]

I think this is okay :)

[bfintal]

I take that back. Now because of the change, you can't add a blank text element, then add an image/video since the focus goes away.

I really think the right approach would be just to force back the caret if it's gone from an empty text element, since that's the problem. The fix has ramifications :( I'll check into how that should be done. I'm rolling back my copy.

[bfintal]

Here's a solution to replace the last commit. I took the approach that I mentioned. To avoid any unwanted effects, I only fixed the exact problem: if the text element is empty, it cannot be clicked on again to bring back the cursor. Sorry, as much as I want to send a PR, it's easier and faster for me to do this is pure JS

Text.prototype._addDOMEventListeners = function() {
    Text.__super__._addDOMEventListeners.call(this);
    return this._domElement.addEventListener( 'mousedown', (function(_this) {
        return function(ev) {
            if ( ! _this.isMounted() ) {
                return;
            }
            if ( _this.content.isWhitespace() ) {
                var selection = new ContentSelect.Range(0, 0);
                return selection.select( _this.domElement() );
            }
        };
    })(this));
};

[bfintal]

Just found out that the previous change also messed with the history :(

[anthonyjb]

Thanks @bfintal - so does this mean that the empty element is retained after focus is lost and that it can then be clicked on again to give it back focus?

[bfintal]

@anthonyjb Yup, exactly :)

[anthonyjb]

        # HACK: If the content of the element is empty and it already has focus
        # then supress the event to stop odd behaviour in FireFox.
        #
        # Anthony Blackshaw <ant@getme.co.uk>, 2016-01-30
        if @content.length() == 0 and ContentEdit.Root.get().focused() is this
            ev.preventDefault()

OK looking a the source there's a reason that we didn't implement it in this way (e.g on click before) - not a super descriptive comment there though so I'm going to have to have a play.

[anthonyjb]

OK so I can confirm this hack is the reason that this bug occurs.

[bfintal]

Ah I remember that. From what I remember though, the issue why that was implemented is similar and may actually be also solved by the above fix.

[anthonyjb]

To be honest I no longer see an issue in firefox if I remove the hack - I'm think about just removing it.

[bfintal]

Reference: #116

[anthonyjb]

Thank you :) That error still occurs - so I need to resolve the original issue without the hack, figure out why firefox behaves this way.

[anthonyjb]

I think as per your code forcing the selection to be 0,0 is the most likely solution btw.

[anthonyjb]

So this code seems to resolve the issue, and looks reasonable neat for a hack to me:

        # HACK: If the content of the element is empty and it already has focus
        # then supress the event to stop odd behaviour in FireFox. See issue:
        # https://github.com/GetmeUK/ContentTools/issues/118
        #
        # Anthony Blackshaw <ant@getme.co.uk>, 2016-01-30
        if @content.length() == 0 and ContentEdit.Root.get().focused() is this
            ev.preventDefault()
            new ContentSelect.Range(0, 0).select(this._domElement)

[bfintal]

+1 I think this is functionally the same. Good that it can be placed in the same hack fix :)

[anthonyjb]

Grrr this still has issue in FF, this is the final hack:

    _onMouseDown: (ev) ->
        # Give the element focus
        super(ev)

        # If the user holds the mouse down for an extended period then start
        # dragging the element.
        clearTimeout(@_dragTimeout)
        @_dragTimeout = setTimeout(
            () =>
                @drag(ev.pageX, ev.pageY)
            ContentEdit.DRAG_HOLD_DURATION
            )

        # HACK: If the content of the element is empty and it already has focus
        # then supress the event to stop odd behaviour in FireFox. See issue:
        # https://github.com/GetmeUK/ContentTools/issues/118
        #
        # Anthony Blackshaw <ant@getme.co.uk>, 2016-01-30
        if @content.length() == 0 and ContentEdit.Root.get().focused() is this
            ev.preventDefault()
            if document.activeElement != this._domElement
                this._domElement.focus()
            new ContentSelect.Range(0, 0).select(this._domElement)

[anthonyjb]

Think that's it fixed - thanks for your help tracking that down, still not happy with the fix as I would like to know why FF displays this behaviour but for another day.