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.

Sign up for GitHub

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

various cleanups while tracking down an issue #186

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

various cleanups while tracking down an issue #186

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dfa0d5c
js/textareas: fail early and with a reasonable message
attila-lendvai Jul 14, 2022
599a0e5
js/contextmenu: rename menu_enabled -> menu_is_installed
attila-lendvai Jul 14, 2022
038fc90
js/xmlcomms: fail early with an assert when no text in message
attila-lendvai Jul 14, 2022
0c86b92
Added more logging while tracking down an issue
attila-lendvai Jul 14, 2022
43eabb6
Fix some typos
attila-lendvai Jul 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions javascript/xmlcomms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats the user visible effect here? Besides you can have empty text areas so what are we trying to catch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

var cmd = msg.msg;
var id = msg.id;
var text = msg.text;
Expand Down