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

Severe input delay with en_GB spellchecking enabled #387

Closed
2 tasks done
timcooper opened this issue Mar 8, 2017 · 7 comments · Fixed by #390
Closed
2 tasks done

Severe input delay with en_GB spellchecking enabled #387

timcooper opened this issue Mar 8, 2017 · 7 comments · Fixed by #390
Assignees
Milestone

Comments

@timcooper
Copy link

timcooper commented Mar 8, 2017

My Setup

Windows 10 Pro
2.5.0 (was the same in 2.4.0)

  • I have tested with the latest application version
  • I can simulate the issue easily

Description

When typing a message there is a severe delay between keystrokes after the first word and the characters rendering in the box (in fact the entire application hangs during this time). There are some older closed issues reporting this with PRs/commits supposedly fixing but this issue is still very much there on the latest version.

Same/similar closed issues:
#289
#358

Quite often this also results in the app freezing completely requiring a restart.

I can always reproduce this by enabling spell checking then starting to type (begins after first word/space). Currently I can't use the app because of this issue and just have rocket.chat open in a chrome window.

Current Behavior

Unusable input delay:
test

Expected Behavior

Works as expected after disabling spellchecking for en_GB:
test2

@andyshilton
Copy link

We can confirm this too. Our Windows 10 users have been experiencing the exact same issue. After turning off the spell checking, it works ok.

@thehutman
Copy link
Contributor

thehutman commented Mar 10, 2017

Windows 10 EN-US spelling isn't quite as bad, but the lag gets progressively worse the more text we write. Doing a performance CPU profile on a long string already in the message box (adding just two words to a 20 word sentence) shows a huge amount of time going on within a few scripts:

image

This was done from the server dev-tools within the application.

Turning off spell-check removes the lag.

The problem doesn't occur when just running within chrome.

@Luxriot-Dev
Copy link

+1

@thehutman
Copy link
Contributor

I'm trying to understand some recent code changes in /src/public/lib/SpellCheck.js I think are causing this problem.

    isCorrect (text) {
        if (!this.enabledDictionaries.length || this.contractions[text.toLocaleLowerCase()]) {
            return true;
        }

        if (this.muliLanguage) {
            for (let i = 0; i < this.enabledDictionaries.length; i++) {
                checker.setDictionary(this.enabledDictionaries[i]);
                if (!checker.isMisspelled(text)) {
                    return true;
                }
            }
        } else {
            return !checker.isMisspelled(text);
        }
        return false;
    }

The part if (this.multiLanguage) seems suspect. In our case only one language is enabled .. so why check multiple? If we modify the check to be something like if (this.enabledDictionaries.length > 1) then the performance seems much better -- when only one dictionary is enabled. If we enable TWO dictionaries, then performance becomes drastically slower when we go into this mode. Seems like repeatedly calling the setDictionary method is a performance killer.

Not really sure how to combat this for anyone using multiple languages (we aren't). Maybe a limitation of the spellcheck library.

@thehutman
Copy link
Contributor

thehutman commented Mar 10, 2017

Ok, looking at how multiLanguage is assigned, it appears to want to indicate FALSE when running on a windows platform and there are no loaded dictionaries. However, the code doesn't work like that I believe:

    loadAvailableDictionaries () {
        this.availableDictionaries = checker.getAvailableDictionaries().sort();
        if (this.availableDictionaries.length === 0) {
            this.muliLanguage = false;
            // Dictionaries path is correct for build
            this.dictionariesPath = path.join(remote.app.getAppPath(), '../dictionaries');
            this.availableDictionaries = [
                'en_GB',
                'en_US',
                'es_ES',
                'pt_BR'
            ];
        } else {
            console.log(isWindows);
            this.muliLanguage = !isWindows;
            this.availableDictionaries = this.availableDictionaries.map((dict) => dict.replace('-', '_'));
        }
    }

In the else block, there is !isWindows -- but this is assigned via:
const isWindows = ['win32', 'win64'].indexOf(os.platform());

This returns a -1 when not a win32/win64 platform. Seems like the multiLanguage needs to be this.muliLanguage = (isWindows < 0) but I am only making an assumption here based on how the spellcheck library appears to operate on my systems here (Win10 64 bit -- which report as os type win32 for some reason). The original code turns on the multiLanguage flag (IsWindows is 0), causing slowness by constantly changing the available dictionaries.

@alexbrazier
Copy link
Contributor

@thehutman good spot. The isWindows variable was just missing !== -1 to make it a bool. I have opened a PR that fixes the issue.

@alexbrazier alexbrazier self-assigned this Mar 11, 2017
@alexbrazier alexbrazier added this to the 2.6.0 milestone Mar 11, 2017
@brandborg
Copy link

brandborg commented Mar 27, 2017

I had the same issue and I can verify after installing the build artifact from the merge request here:
https://ci.appveyor.com/project/RocketChat/rocket-chat-electron/build/1.0.289/artifacts
That it's no longer an issue for me!
Windows10 Pro
EN_uk keyboard
Version: Rocket.Chat 2.6.0-develop

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 a pull request may close this issue.

6 participants