various cleanups while tracking down an issue #186
Conversation
...when the contextmenu event handler is not triggered for whatever reason.
This is a rebase of PR 148. Signed-off-by: Christian Krause <christian.krause@mailbox.org>
| @@ -102,6 +102,7 @@ chrome.commands.onCommand.addListener(function(command) { | |||
| function handleContentMessages(msg, tab_port) | |||
| { | |||
| console.log("handleContentMessages called:"+JSON.stringify(msg)); | |||
| console.assert(msg.text); | |||
There was a problem hiding this comment.
whats the user visible effect here? Besides you can have empty text areas so what are we trying to catch?
There was a problem hiding this comment.
empty text areas have an empty string as their content. this is checking for null.
it happens when the contextmenu event listener is not triggered, and the message contains no text. (or worse: with HEAD it uses the previous content)
the user visible effect is that nothing happens, besides the log in the console. not ideal, but IMO better than the previous behavior. at least people with knowledge can hone in much faster on the actual issue.
also, i'm suggesting to get rid of the current console.log stuff, and use console.debug where appropriate, and let the users decide in their console config how much log they want to see.
it took me hours to understand what was happening. with these changes it should be minutes at most.
|
What is this PR trying to achieve? I don't really want to merge a bunch of ancillary code motions and debug logs unless they are going to be useful in the long term. What is the actual fix in here? |
it fixes nothing. what it does is it greatly improves debuggability. the issue that needs to be fixed is that the |
various cleanups while tracking down why it doesn't work on https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_textarea