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

To repair the open image dialog is very slow in Chrome #189

Closed
raisezhang opened this issue Apr 14, 2017 · 13 comments
Closed

To repair the open image dialog is very slow in Chrome #189

raisezhang opened this issue Apr 14, 2017 · 13 comments

Comments

@raisezhang
Copy link

raisezhang commented Apr 14, 2017

This issue mainly solve the quill in chrome52+ image dialog is very slow bugs!

In fact this problem the quill trunk has been repair the question. slab/quill#1265, slab/quill#1260
But the quill is not yet released the latest code to the NPM and the react-quill isn't reference to the latest code.

So to solve this problem, need to temporary hack, the code is as follows:

/*
 *  [email protected]
 */

class MyComponent extends React.Component {
    constructor(props) {
        super(props);
        this.state = {
            text: ''
        }
        this.qlToolBar = null;
    }

    changeInputAttribute(e) {
        if (e.target.className.indexOf('ql-image') >= 0) {
            e.target.setAttribute('accept', 'image/png, image/gif, image/jpeg, image/bmp, image/x-icon, image/x-quicktime');
        }
    }

    componentDidMount() {
        this.qlToolBar = this.refs.quillEditor.getEditor().container.parentNode.querySelector('.ql-toolbar');
        this.qlToolBar.addEventListener('DOMNodeInserted', this.changeInputAttribute, false);
    }

    componentWillUnmount() {
        if (this.qlToolBar) {
            this.qlToolBar.removeEventListener('DOMNodeInserted', this.changeInputAttribute, false);
            this.qlToolBar = null;
        }
    }

    handleChange(value) {
        this.setState({text: value})
    }

    render() {
        return (<ReactQuill ref="quillEditor" value={this.state.text} onChange={this.handleChange}/>)
    }
}

reference: https://github.com/YouHan26/quill/blob/494d87b7388ca9d6e65ee396980f0554b7edb40f/themes/base.js#L128

@alexkrolick
Copy link
Collaborator

Thanks for documenting the workaround.

1.0.0-rc.2 has quill^1.2.0 as a dependency, which looks like it has the fix from slab/quill#1090: https://github.com/quilljs/quill/blob/v1.2.0/themes/base.js#L127

@raisezhang
Copy link
Author

image/svg+xml also need to be deleted.
https://github.com/quilljs/quill/releases

@alexkrolick
Copy link
Collaborator

alexkrolick commented Apr 16, 2017

I don't see a significant different in open time for the image dialog between the handler w/out SVG and the default.

I'm on Chrome 57.

cc @jhchen, slab/quill#1265

@jhchen
Copy link

jhchen commented Apr 16, 2017

I have the same concern which is that I also do not experience and noticeable slowdown or speedup with the svg removal whereas the issue reports a 6-10s slowdown. I was able to reproduce the original issue slab/quill#1090 in addition to the issue being well documented on StackOverflow and Chrome bug reports and promptly accepted the PR.

@raisezhang
Copy link
Author

Thanks @jhchen @alexkrolick

@jhchen
Copy link

jhchen commented Apr 17, 2017

@raisezhang How long does it take you to open the default and non-SVG file dialogs that @alexkrolick posted?

@raisezhang
Copy link
Author

raisezhang commented Apr 17, 2017

default: about 8 seconds
http://recordit.co/YS1oLDicqP

non-SVG: about 1 seconds
http://recordit.co/ix4a3blEjF

macOS Sierra 10.12.2
Chrome 57.0.2987.133 (64-bit)

@alexkrolick
Copy link
Collaborator

Ok thanks for the demo - this is Chrome 57 on Windows I'm assuming?

@alexkrolick
Copy link
Collaborator

This actually a Chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=638874

@jhchen what's your policy on working around platform bugs?

@alexkrolick
Copy link
Collaborator

@raisezhang if anyone feels the need to mitigate the issue before Chrome fixes it they should be able to follow your patch.

@jhchen
Copy link

jhchen commented Apr 21, 2017

I don't think the linked Chrome bug is describing this issue. There was previously an Issue with the same side effect slab/quill#1090 but this Issue has to do with svgs specifically.

@alexkrolick
Copy link
Collaborator

The SVG-specific bug is tracking here, although not much progress has been made: https://bugs.chromium.org/p/chromium/issues/detail?id=678507

@jhchen
Copy link

jhchen commented Apr 21, 2017

Okay thanks for the link. In any case it Quill's policy is to not rely on browsers to fix anything if possible and to work around or fix in another layer. In some ways the existence of Quill owes to the fact that browsers have not fixed the many issues surrounding contenteditable.

alexkrolick referenced this issue in slab/quill Apr 22, 2017
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

3 participants