d1vanov/libquentier

Infinite loop in Replace all

Closed this issue · 5 comments

Easily reproduces by input a same word into the find and replace fields. Then press Replace all button - Note editor hangs and app crashes on exit.
Actually, the words might be not same but similar, like 'wake' and 'awake'.

minidump.tar.gz

src/note_editor/javascript/scipts/findReplaceManager.js

    this.replaceAll = function(textToReplace, replacementText, matchCase) {                                             
        console.log("replaceAll: text to replace = " + textToReplace +                                                  
                    "; replacement text = " + replacementText + "; match case = " + matchCase);                         
                                                                                                                        
        observer.stop();                                                                                                
                                                                                                                        
        try {                                                                                                           
            var counter = 0;                                                                                            
            while(this.replace(textToReplace, replacementText, matchCase)) {                                            
                ++counter;                                                                                              
            }                                                                                                           
                                                                                                                        
            undoReplaceAllCounters.push(counter);                                                                       
        }                                                                                                               
        finally {                                                                                                       
            observer.start();                                                                                           
        }                                                                                                               
    }

Need to somehow modify this function to ensure it won't run on the same text over and over again; this.replace function called in a loop uses window.find which doesn't seem able to start from some offset within the text so probably replaceAll should use something different - regexp maybe.

I'll try to fix this one.

Be my guest! Just in case, I've got one idea how one could approach fixing this: add one more argument to this.replace which would be the number of matches found by window.find to skip until doing the actual replacement; the method would consider this parameter if and only if replacementText contains textToReplace within it.
Also would be nice to add special processing doing nothing if replacementText is equal to textToReplace.

This feels hacky so you're pretty welcome to try the regexp approach instead, however, it might have issues with Unicode, for example, with Cyrillic text.

I've just checked, this code works with the Cyrillic text:

var re = new RegExp(textToReplace, 'ig');
document.body.innerHTML = document.body.innerHTML.replace(re, replacementText);

This one was fixed via PR #47.