regex-search icon indicating copy to clipboard operation
regex-search copied to clipboard

Search result corrupts HTML page

Open valtih1978 opened this issue 8 years ago • 7 comments

You revoke the corruption when search text changes and Esc pressed. But, Esc pressing just closes the popup and does not revoke the match results in my case. I have to enter some text into the search bar to remove it before closing.

valtih1978 avatar Jul 09 '16 14:07 valtih1978

I have debugged a little and noticed that you catch the messages in content.js. The code

chrome.runtime.onMessage.addListener(function (request, sender, sendResponse) {

receives cleanup message when you type some search string but nothing arrives on Esc, despite you send the clean.

valtih1978 avatar Jul 09 '16 15:07 valtih1978

The easiest solution that worked for me was to

  1. stop doing anything on Esc
  2. add a Close button to the popup with the following event
        closeButton.addEventListener("click", function () {
            clear(); setTimeout(function(){window.close()}, 0)
        })

where clear() is

        function clear() {
//            if (tabStates.isSearching(id)) {
                setSearching(id, false, tabStates);
                Utils.sendCommand("clear");
//            }
        }

start using it whenever you need setSearching(false);send('clear'); instead of duplicating your code.

valtih1978 avatar Jul 10 '16 09:07 valtih1978

The working alternative that would automatically clean the search one popup closed is to use a timer instead of the Close button. You send the ping messages periodically from popup.js to the content.js. When "search" is received, content.js starts a timer, which runs clear() on timeout. The timer is restarted on arrival of ping. This will postpone the clean until popup is really closed. Content.js will detect this by the timeout monitor and clean up the search. I did, it, works.

valtih1978 avatar Jul 11 '16 18:07 valtih1978

So when the search bar is focused and I hit Esc, the search clears as expected. When I click the "Next" button and then hit escape, only the popup closes, and the search stays. I think the solution here would just be to add a keypress handler on both buttons that clears when Esc is pressed.

gsingh93 avatar Jul 13 '16 01:07 gsingh93

At first, it is false. I hit enter to search, focus stays in the input input. I also mouse click there to ensure that and hit Esc. Only popup but no search results disappear. My observation that popup closes unpredictably, when I am debugging the esc event in the popup window, suggests that there is a race condition. It is just Chrome design. Chrome closes your popup whenever another window gets the focus. In this light, when you cannot even ensure proper cleanup when user clicks your beloved Esc, your desire to catch all close events with the Esc or Next key down looks funny.

The only reliable keydown design is a special Close button that will both clear the result and close the popup window. I find it however more practical to clear by ping timeout eventhough, theoretically, under high loads, ping msg may be delayed by more that 500 ms in the presence of active popup.

valtih1978 avatar Jul 13 '16 06:07 valtih1978

If we decide to clear the search on any close of the popup, then we should probably do something like this.

gsingh93 avatar Jul 13 '16 07:07 gsingh93

If we decide to clear the search on any close of the popup, then we should probably do something like this.

Chrome fails to generate window.unload event when popup is closed We must use ports and polling instead

This is actually the workaround that I used. My timeout solution uses both polling (ping) and ports (send message).

valtih1978 avatar Jul 13 '16 07:07 valtih1978