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

Search result corrupts HTML page #34

Open
valtih1978 opened this issue Jul 9, 2016 · 7 comments
Open

Search result corrupts HTML page #34

valtih1978 opened this issue Jul 9, 2016 · 7 comments

Comments

@valtih1978
Copy link

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 valtih1978 changed the title Search result corrupt HTML page Search result corrupts HTML page Jul 9, 2016
@valtih1978
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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 pushed a commit to valtih1978/regex-search that referenced this issue Jul 12, 2016
@gsingh93
Copy link
Owner

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.

valtih1978 pushed a commit to valtih1978/regex-search that referenced this issue Jul 13, 2016
@valtih1978
Copy link
Author

valtih1978 commented Jul 13, 2016

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.

@gsingh93
Copy link
Owner

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

@valtih1978
Copy link
Author

valtih1978 commented Jul 13, 2016

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).

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

No branches or pull requests

2 participants