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

Focus issues when scrolled down in IE and FF #221

Closed
lewisvance opened this issue Sep 25, 2014 · 13 comments
Closed

Focus issues when scrolled down in IE and FF #221

lewisvance opened this issue Sep 25, 2014 · 13 comments

Comments

@lewisvance
Copy link

You can reproduce this right on the demo page: http://quilljs.com/examples/
Add some new lines and some text so scroll bars appear. Scroll to the bottom, select some text and click bold on the toolbar. The editor will jump to the top hiding the text you selected. IE11 is much worse in that single or double clicking in the editor causes it to jump around.

Master has the issue as well.

Tested in IE11 and FF 32.0.3. Chrome 37.0.2062.124 does not have the issue.

@jhchen
Copy link
Member

jhchen commented Sep 26, 2014

I was able to reproduce on IE11 but I'm not seeing it in FF on either Windows or Mac. What OS and version did you encounter this on FF?

@lewisvance
Copy link
Author

FF on mac although I'm trying to repro it again without success.

@jonathansampson
Copy link

I work on the Internet Explorer team and was able to repro the issue locally. I'll be setting aside some time tomorrow to investigate further and will be happy to update this issue if/when I come across meaningful data. I have also taken the liberty to inform our text/input Program Manager of the issue, just in case it resembles anything else we're aware of internally.

@jhchen
Copy link
Member

jhchen commented Oct 14, 2014

Awesome thanks @jonathansampson!

@scottjacobsen
Copy link

I can re-create on FF for mac. Put in at least 100 lines of text, scroll to the bottom, and hit bold, italic, or probably anything. It will scroll back up. It doesn't scroll up if you hit cmd-b.

I haven't been able to reproduce with chrome or safari on mac.

@scottjacobsen
Copy link

So far I've tracked it down to calling focus() on the editor div.

When clicking a formatting link we hit this:
https://github.com/quilljs/quill/blob/0a669f05844086e403f6ac6f016f0c0e0ae0dd1e/src/lib/dom.coffee#L127

Then we hit @quill.focus()
https://github.com/quilljs/quill/blob/0a669f05844086e403f6ac6f016f0c0e0ae0dd1e/src/modules/toolbar.coffee#L67

I can confirm that just calling focus on the editor scrolls it to the top, but only if the document is long enough and you are scrolled down far enough. Excuse the JQuery, but just run something like this in the console:
$($('iframe')[0]).contents().find('#quill-1')[0].focus()

And it will scroll to the top.

@scottjacobsen
Copy link

This commit fixes it for FF - 0a669f0.

Focusing the iframe before focusing the editor div seems to do it. From what I can tell focusing the iframe is sufficent to focus the editor div, so maybe there is no reason to call @root.focus().

It does not fix it for IE11 though.

@jhchen jhchen closed this as completed in 815fd32 Nov 6, 2014
@scottjacobsen
Copy link

I just cherry-picked this one in. It fixes IE 👍, but breaks it for FF 😢

@scottjacobsen
Copy link

The key for FF is to focus the iframe first. This appears to work for me in FF, IE11 and Chrome:

  focus: ->
    if @selection.range?
      @selection.setRange(@selection.range)
    else
      @renderer.iframe.focus()
      @root.focus()

@scottjacobsen
Copy link

Actually my fix isn't working with FF. Still trying to figure it out.

@scottjacobsen
Copy link

I believe my latest commit - scottjacobsen@d34b3cf21b7a08d229e64f0ef4e2f59ddaa567ea works for the trifecta of desktop browsers IE11, FF, and Chrome.

@scottjacobsen
Copy link

Just realized you are removing iframes, so that stuff about FF and iframes doesn't apply to the latest. Does it make sense to do a 0.18.2 maintenance release with this fix?

@jhchen
Copy link
Member

jhchen commented Nov 6, 2014

The iframe removal has already been merged into the develop branch and I'm planning to release very soon so it would not be trivial to sneak in a maintenance version.

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

No branches or pull requests

4 participants