Ticket #5830 (review_failed Bug)

Opened 4 years ago

Last modified 3 years ago

Scayt: the browser hangs when large(?) documents (50KB+) are loaded

Reported by: wwalc Owned by: garry.yao
Priority: Normal Milestone:
Component: UI : Spell Checker Version: 3.4.2
Keywords: Review? Cc: SpellChecker.net, mrfr0g, mcamden@…

Description

Imho this might be a serious issue as the editor should be working fine out of the box without having to dig into the documentation for specific settings that might speed up the editor performance or to simply make it working.

The problem: when having a larger document, scayt is causing the "script to run slow". This looks pretty nasty, especially if the document is not really that large (for example: 50KB), also many users will have no idea whether to stop the script execution and will simply consider the editor as broken.

I have attached two sample documents that can be used to reproduce this issue:

  • English document (380KB+)
  • Polish document (50KB)

I guess that Scayt does not recognize that the text is in different language than English and is finding too many errors in Polish document, that's why it hangs even when editing small documents.

We should either:

  • disable Scayt on startup
  • fix it to perhaps find just first 20/50(?) errors and then give up and display a gentle error message or a warning icon somewhere that there are more than 50 errors found, at least when it starts automatically. When user clicks on the button and enables Scayt manually, we might display an information to be patient and that it may take a while if the document is large.

Attachments

scayt_issues.zip (109.3 KB) - added by wwalc 4 years ago.
5830.patch (907 bytes) - added by SpellChecker.net 4 years ago.
5830.2.patch (907 bytes) - added by SpellChecker.net 4 years ago.
Patch on TooManyMisspellings and BigText
5830.3.patch (2.6 KB) - added by SpellChecker.net 3 years ago.

Change History

Changed 4 years ago by wwalc

comment:1 Changed 4 years ago by fredck

  • Milestone set to CKEditor 3.4

comment:2 Changed 4 years ago by SpellChecker.net

As the first step in solution of this problem SpellChecker.net introduces maxSizeMarkup setting which defines threshold size in symbols of editable element content. If the size of the text is larger then specified value SCAYT fires event which is handled by plug-in code. Now plug-in switch off SCAYT in case of such event. The default value of maxSizeMarkup is 30000. It can be changed by adding following code to the page.

scaytConfig = {
	maxSizeMarkup: 5000
};

As further steps we plan to

  • introduce tooManyMisspellings and/or incorrectLanguage events that will disable SCAYT (SCAYT core and plug-in).
  • introduce signalization to the end-user and processing end-user input (plug-in).

the patch attached requires patch #5923 to be applied

comment:3 Changed 4 years ago by fredck

  • Milestone CKEditor 3.4 deleted

Changed 4 years ago by SpellChecker.net

Changed 4 years ago by SpellChecker.net

Patch on TooManyMisspellings and BigText

comment:4 Changed 4 years ago by SpellChecker.net

Sorry, double click. Please remove 5830.2.patch.

comment:5 Changed 4 years ago by SpellChecker.net

  • Keywords HasPatch, Review? added
  • Version changed from 3.3 to 3.4.1 (SVN - trunk)

Here is the patch, which disables SCAYT when: a) there are too many misspelled words in the text (e.g. incorrect language); b) the text size is significantly big; We have added two new events to the SCAYT core and implemented their support in the plugin code. We also introduced three new parameters:

  1. maxSizeMarkup - if text contains more words than this value, SCAYT will be disabled.
  2. minCheckIncorrectWord - minimum number of words, when check for percentage of incorrect words will be held
  3. maxPercentIncorrectWord - maximum percent of incorrect words in text, if percentage of incorrect words in the whole text is more than this value SCAYT will be disabled.

For minCheckIncorrectWord default value is 10, for maxPercentIncorrectWord default value is 0.8, for maxSizeMarkup default value is 6000. You can override this settings like this: scaytConfig = {

minCheckIncorrectWord: 20, maxPercentIncorrectWord: 0.5, maxSizeMarkup: 5000

}; In future SpellChecker.net team plans to introduce some kind of signalization to the end-user and processing end-user input.

comment:6 Changed 3 years ago by garry.yao

  • Status changed from confirmed to review
  • Owner set to garry.yao
  • Keywords HasPatch added; HasPatch, Review? removed

I'm not seeing the "disabling" on behavior, SCAYT still fills the initial request with fully document thus the button state takes long to activate, is that how it should work?

Besides, those new configuration options should appears in the patch as documentation.

comment:7 Changed 3 years ago by garry.yao

  • Status changed from review to review_failed

comment:8 Changed 3 years ago by SpellChecker.net

SCAYT just becomes disabled if number of misspelled words is greater than allowed. You can't enable it until it has more misspelled words than allowed.

We have no UI signalization of this event for now. In future we plan to implement different icons for SCAYT button in toolbar to reflect its state and maybe to comment some events (e.g. "SCAYT is disabled as there are too many misspelled words").

We would also like to know your vision about how this feature should be implemented.

comment:9 Changed 3 years ago by garry.yao

  1. A threshold approach doesn't resolves the real problem as the first request still carries full list of words in such a large document that may hang the browser as well.
  2. Even saying that we accept the threshold approach, it should not be measured by counting of words, but by a time slot that browsers need to deliver the marking up.

With the ticket TC (large_pl.html), the browser hangup is still experienced for me.

comment:10 Changed 3 years ago by SpellChecker.net

  • Keywords HasPatch, Review? added; HasPatch removed
  • Version changed from 3.4.1 to 3.4.2

Here is the updated patch. Now SCAYT doesn't not wait until all words are send to server, but checks the number of incorrect words in each requests. If there are too many misspelled words in the first response, then SCAYT becomes disabled.

Example: For large_pl.html file about 4 or 5 requests are send to server. Previously SCAYT waited for all requests to send and then checked for misspellings. But now it disables after first request, as there are too many misspelled words in it.

Please note, that this will work only after updating SCAYT to version 2.6.0 with this task: http://dev.ckeditor.com/ticket/6769

Also, description of settings for tooManyMisspellings and BigText features were added to plugin.

Changed 3 years ago by SpellChecker.net

comment:11 Changed 3 years ago by garry.yao

  • Keywords HasPatch, Review? removed

We can't really disable SCAYT just because of one document snapshot, user should be able to turn it on at any time, besides, the following are two status which conflict.

this.setDisabled( true ); 
editor.getCommand( commandName ).setState( CKEDITOR.TRISTATE_OFF );

For us, it's not appropriate to abuse configurations with entries that doesn't really benefit the users:

  1. "minCheckIncorrectWordInQuery" : user should have no knowledge about a query at all because he don't even know how much you fill a query, don't he? It's enough to keep "minCheckIncorrectWord" for us.
  2. "maxPercentIncorrectWord" and "maxPercentIncorrectWordInQuery" : this means rejecting the user because he/she types too much misspells, a prejudice?

comment:12 Changed 3 years ago by fredck

  • Cc mrfr0g, mcamden@… added

#7623 has been marked as DUP.

comment:13 Changed 3 years ago by SpellChecker.net

  • Keywords Review? added

With release of SCAYT Core v2.6.4 (1058) WebSpellChecker.net introduced new approach to large text processing. Specified release contains initial improvements related to preloaded document. Now preloaded document with size 50KB+ doesn't cause "script to run slow" error.

Note: See TracTickets for help on using tickets.
© 2003 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy