nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
https://www.nvaccess.org/
Other
2.09k stars 631 forks source link

Change "Restart with add-ons disabled" to also set log level to debug #11538

Open Qchristensen opened 4 years ago

Qchristensen commented 4 years ago

Is your feature request related to a problem? Please describe.

The main use case of NVDA's "Restart with add-ons disabled" option (in the list when NVDA+Q is pressed), is to troubleshoot problems. When troubleshooting problems, it is also useful to have debug logging enabled.

Describe the solution you'd like

I suggest having this option restart with add-ons disabled, AND the log level set to debug for this instance.

A couple of ways this could be done:

Describe alternatives you've considered

Currently, when troubleshooting an issue with a user, I request that they enable debug logging manually:

  1. Press NVDA+control+g
  2. Press tab to "logging level"
  3. Press down arrow to "Debug"
  4. Press enter to set
  5. Press NVDA+control+c to save

Having this level of logging enabled automatically when restarting with add-ons disabled would make that process much easier for users who may have varying levels of skill.

Additional context

There are likely a few cases where restarting with add-ons disabled may be used NOT in a troubleshooting situation. In this case, increasing the log level should not pose any problem. In terms of privacy, as always, the log is never sent anywhere unless the user manually locates the file and sends it somewhere, and I believe displaying a warning at the start should help alleviate any potential concerns over the logging level being changed.

My thought over adding a warning is also that it means the text of the option ("Restart with add-ons disabled") does not need to be changed - which saves it from becoming quite long, and potentially confusing.

XLTechie commented 4 years ago

I like this idea.

As someone who often supports users, this is the most common set of first steps that need to be explained, and when done over the phone or the like, talked through.

The only possible alternative I might suggest, would be to add a new option called "Restart in debug mode" or something similar, instead of modifying the current option. That option would restart with add-ons disabled, and log level set to debug.

In the future, it could be expanded to perhaps do things like capture core dumps on restart (like @derekriemer's Crash Hero add-on does), and maybe more optimizations for trouble shooting that I'm not thinking of right now.

Qchristensen commented 4 years ago

I was aiming for the simplest solution, but certainly looking at the restart options, the quit dialog currently has four options:

If we were to have a "Restart in debug mode" option, I'd be inclined to have that replace the last two options. It would mean that in order to restart with add-ons running and debug logging, you'd need to set that from the general settings screen. The people who may need that are those testing add-ons who are likely advanced enough that the extra steps to change the logging level specifically should not be a problem.

Certainly either way would resolve the issue. Replacing "Restart with add-ons disabled and "restart with debug logging enabled" with one option would remove the possibility of someone going down one too many options when trying to troubleshoot and accidentally restarting with debug logging but add-ons still enabled.

CyrilleB79 commented 4 years ago

I fully agree with this feature request: a simple method to restart with add-on disabled and log level set to DEBUG is clearly missing in NVDA's core.

It would mean that in order to restart with add-ons running and debug logging, you'd need to set that from the general settings screen. The people who may need that are those testing add-ons who are likely advanced enough that the extra steps to change the logging level specifically should not be a problem.

No, this is a wrong assumption. As an add-on author, I may ask users experimenting some issues to provide a log on debug level normally and of course add-on activated. So there are at least two use cases for simple users:

If no other use case is identified, I would be inclined to keep 4 options, but to replace "Restart with add-on disabled" by "Restart in debug mode" (add-on disabled, log level = DEBUG and maybe something else one day as suggested by @XLTechie)

I had also in my mind to create an add-on providing a GUI to restart specify any command line parameter, for advanced test purpose. Anyway, these would be targeted for advanced tester and thus not adapted for NVDA core. We should keep in mind that the added or modified options should remain dedicated to simple users who are requested to perform simple tests.

Replacing "Restart with add-ons disabled and "restart with debug logging enabled" with one option would remove the possibility of someone going down one too many options when trying to troubleshoot and accidentally restarting with debug logging but add-ons still enabled.

Well, selecting the wrong option does not seem a problem for me. If a user is instructed to provide a log and selects the wrong option, this will be visible in the log.

josephsl commented 4 years ago

Hi, at least for installed copy, I think it would be much better to tell users to pass in command-line switches from Run dialog, as both options can be done in one sitting. As for number of options in Exit NVDA dialog, it is actually five – don’t forget “install pending update”. In either case, because my add-ons do check for debug logging flag and is coded to support debug logging, I’m flexible regarding what the direction our discussion will take and am ready to make necessary changes.

CyrilleB79 commented 4 years ago

Hi, at least for installed copy, I think it would be much better to tell users to pass in command-line switches from Run dialog, as both options can be done in one sitting.

Sorry, I disagree again. The real question is when should we provide a GUI option in the exit dialog and when is a command line flag enough. IMO:

A basic user reporting (e.g. on a mailing list) that NVDA or an add-on does not work should be given instructions as simple as possible. I do not consider the command line simple and user-friendly for a basic user.

josephsl commented 4 years ago

Hi, suppose Quentin’s suggestion gets implemented (no add-ons yet debug logging is on). Will this help users troubleshoot add-on issues given that the actual messages that we (users and developers) are looking for will not be logged? The alternative then is telling users to check logging level from general settings panel, which I think should be something beginners may not understand at first. Besides, unless users are accustomed to it, restarting with debug logging on WITH add-ons turned on is done after consulting add-on developers. The command-line option I mentioned is just one way of achieving this provided that developers and users come to an agreement that this step should be tried. In the end, I think what makes a difference is not combining options for sake of making it easier for people to troubleshoot issues, but rather helping people describe issues a bit better and training add-on authors on communicating better with users, because I think feedback, not options, makes or breaks something. Thanks.

josephsl commented 4 years ago

Hi,

Expanding on what I wrote above:

I think this proposal describes a tendency that we (users and developers) find ourselves in from time to time: trying to sort issues with add-ons in a way that is easy for everyone. While I agree with this premise, I believe that there is a bigger topic to discuss: actual communication between users and developers.

When a user posts a bug report about NVDA, if the issue stems from an add-on, we would advise users to disable the affected add-on. As noted several times, currently disabling add-ons do not touch logging level whatsoever unless specific steps are taken. Only when add-on authors or power users advise people to restart with debug logging enabled, or when the logging level is changed to "debug" will add-ons log debug information. At least this helps developers understand what's going on provided that they have expert knowledge on their add-ons.

Now suppose that as Luke and Quentin (and many others) pointed out, we combine a way to get debug information while add-ons are disabled. If I understand it correctly, this is same as setting logging level to debug and then restarting NVDA with add-ons disabled. Quentin's alternative (setting log level from general settings panel) is a viable procedure provided that users understand what's going on, but then we circle back to the status quo. A compromise might be a confirmation screen when "restart with debug logging" is selected, asking users if add-ons should be disabled. But I think it doesn't touch the heart of the matter: effective debugging and discourse surrounding it.

Because of this, I think we may need to think about possible factors that have led to this proposal. Certainly ease of debugging is one factor, but then I would like to carefully say that debugging is not easy as people may appear to suggest. Setting up for debug mode, while possible, is just one step in effective add-on debugging - an effective add-on issue debugging involves good (or rather, thorough/expert level) knowledge about an add-on (or two), and dedication to think for a long time and come up with solutions and balancing pros and cons.

Not only that, I think effective communication and feedback loop between users and developers is key, as that is one of the visible results of effective debugging (the most visible result being the bug fix and feedback afterwards). Getting a debug log is one thing, even when using the proposal noted here. But developers should be willing to "paraphrase the bug" i.e. explain the reported bug in terms developers can internalize, then think about causes, effects, solutions, and possible regressions and issues once the chosen solution is deployed, and an added bonus of explaining all this in a user-friendly way (trust me, this takes time to develop).

As I noted above, whatever the outcome will be, I'm willing to adopt, but I think it would be good to consider the paths that led us to this proposal, because the issue of effectively debugging add-on issues cannot be solved by renaming exit routes alone.

Thanks.

CyrilleB79 commented 4 years ago

Huh. Not sure to have understood all your words Joseph, sorry. Anyway, reading you and thinking again to this issue, I can see 3 use cases.

  1. A user encounters an expected behaviour using NVDA: An easy way to restart NVDA with add-on disabled allow to discriminate easily if he should report the issue to NVDA or to add-on authors.
  2. A user reports an issue to NVDA (add-ons disabled confirms NVDA's responsibility): An easy way to restart NVDA with add-ons disabled and log level on DEBUG should help the user to provide a log with useful information to NVDA developers.
  3. A user reports an issue to an add-on author or maintainer regarding his add-on. An easy way to restart NVDA log level on DEBUG (but add-ons enabled, ideally only one) should help the user to provide a log with useful information to NVDA developers.

Maybe there are other use cases, feel free to comment.

Note that in use case 1, to discriminate the add-on responsible of the issue, there should be a way to restart NVDA with only one add-on enabled; but that's a separate issue.

Of course, as you pointed out Joseph, it's just a part of the problem. And a good communication with a basic user is required to understand all the steps that were executed, the conditions, and also to give clear instructions if tests are required.

DrSooom commented 4 years ago

Suggestion:

These two checkboxes are only selectable if the radio button "Restart" was selected before. Thus the options names aren't so verbose and the process is easier to explain to beginners. And additional restart options (e.g. "Use no braille display") can be easily added later in the list of checkboxes. This also means that combining restart options is much easier than using dropdown list entries, which is the case yet.

DrSooom commented 4 years ago

See also: Issue #9686

lukaszgo1 commented 4 years ago

I'm personally a big fan of @DrSooom 's idea from the commend above. It'd be very easy to explain to beginners how to restart with desired options and at the samae time give power users to restart with exactly parameters they need.

Qchristensen commented 4 years ago

Firstly, I agree, I hadn't considered users testing issues for an add-on author, so having the ability to restart with add-ons enabled and debug logging is definitely valid. I was thinking re the suggestion for restarting with just ONE add-on enabled, it might be best to add that to the add-on manager screen - it already gives you a list of add-ons, so it would be easy to select the one you want in that list, then press a keystroke / activate a button to restart with just that add-on enabled.

Continuing that thought, if that were possible, would there still be a need for an exit screen option to restart with ALL add-ons enabled and debug logging?

I still like the one drop down list for the added simplicity over a drop down list and checkboxes, but either way is definitely workable, and easier than asking a user to go into the settings, change debug level, then exit and select an option there.

XLTechie commented 4 years ago

Note that this issue is related to, though not identical with, #9686 , if that hasn't already been pointed out.

@Qchristensen wrote:

Firstly, I agree, I hadn't considered users testing issues for an add-on author, so having the ability to restart with add-ons enabled and debug logging is definitely valid. I was thinking re the suggestion for restarting with just ONE add-on enabled, it might be best to add that to the add-on manager screen - it already gives you a list of add-ons, so it would be easy to select the one you want in that list, then press a keystroke / activate a button to restart with just that add-on enabled.

I agree, both for the use case, and where it should be done. However, #11317 seems relevant here for some cautions about scope.

Continuing that thought, if that were possible, would there still be a need for an exit screen option to restart with ALL add-ons enabled and debug logging?

If that option was implemented, maybe not. But that one seems more complex (see the above referenced issue), and seems like it might meet with much more friction.

I still like the one drop down list for the added simplicity over a drop down list and checkboxes, but either way is definitely workable, and easier than asking a user to go into the settings, change debug level, then exit and select an option there.

Agreed on both points. I don't personally mind a dropdown with more than a few options to cover the possibilities (or probabilities). But if it seems that the radios and checkbox mode is preferable, it would still be better than the current arrangement.

Another possibility might be something like this:

Keep exit, restart, and Install Pending Update as menu items. But make restart a split button (similar to what you proposed for the exit button on the main menu some time ago). If it is pressed, it just restarts. but if it is expanded, you can choose from menu options such as "Debug logging with add-ons", "Debug logging with add-ons disabled", and whatever else needs to go there.

josephsl commented 4 years ago

Hi, I think the last point is a good compromise, although we have to think about feasibility with wxPython. Thanks.

CyrilleB79 commented 4 years ago

Hi

There are good ideas in all proposition that were made. The split button is interesting. But being a less common UI element than basic buttons, radio buttons, dropt-down list or checkboxes, it may require additional explanations for some users (again in the case of a basic user instructed to perform additional tests). I prefer the drop-down list to radio-buttons because it takes less space on the screen and because users are already used to it in NVDA'exit dialog and in Windows exit dialog (pressing Alt+F4 from the desktop). Anyway, I am happy anyway if split-button or radio buttons are adopted.

An other interesting option that I prefer would be:

What do you think of this last proposal?

XLTechie commented 4 years ago

Given the premiss that:

Any user who is restarting with anything other than the normal restart option, is likely to be doing so in order to solve some sort of problem.

It therefore seems reasonable that all non-generic restart options can safely have debugging as their log level. Personally I don't think a warning dialog is required for this, because as @Qchristensen stated: nothing is done with the log without further user action.

There are currently four constant options, plus one part-time option, on the exit menu.

Since @josephsl seemed to suggest there might be WX problems with a split button in this context (which I haven't investigated but will take his implication for), and because @CyrilleB79 may have a point about split button familiarity (which was my own objection in #9686), I will revise my suggestion.

Going a bit back to basics from radio buttons, checkboxes, split buttons, and all that, I suggest:

  1. Replace the exit dialog with a menu. If a menu is not adequate because of the "Add-ons are disabled..." warning, use a dialog with only buttons.

Either will enable us to give a keyboard shortcut to each of its options. In turn, that will make it dead simple to explain steps to inexperienced users.

  1. Make the first option on that menu be: Install Pending Update. Reasoning: if a user is exiting quickly, they may not otherwise notice it if it is not in top position.

  2. The full menu would look like this, first option hidden if not applicable. Keys with alt if the dialog-with-buttons is used:

Install Pending Update (i)

Exit (x)

Restart (r)

Debugging Restart, Add-ons Disabled (d)

Debugging Restart, Add-ons enabled (e)

XLTechie commented 4 years ago

Debugging Restart, Add-ons Disabled (d)

Debugging Restart, Add-ons enabled (e)

I should say that I had four goals with this wording:

  1. Include that a restart was going to happen.
  2. Include that this was for debugging, thereby implying that logging would be different (elaborated in the manual).
  3. Explain what would happen regarding add-ons.
  4. Keep it short.

in truth, 1 and 3 are most important here, and other wording options are available.

The other choices I came up with, though maybe others have better ones, are:

Debugging restart without add-ons Debugging restart with add-ons

And:

Restart without add-ons (debug logging) Restart with add-ons (debug logging)

(Though longer, these latter are my preference over all.)

mzanm commented 2 years ago

Hello, I agree a dropdown list with exit and restart and then when restart is chosen 2 checkboxes for disable addons and enable debug logging is a good idea, because you can choose to do both or one of them easily instead of adding an option for each possibility.

mzanm commented 2 years ago

Hello, is it ok if I work on this? Thank you.

josephsl commented 2 years ago

Hi,

I'm not NV Access employee, but I would say it would be good to play with at least a prototype/pull request artifact. That way we can have something to think about as we discuss this issue further and revisit our comments from 2020, taking into account the differences between the add-ons environment between then and now (January 2022).

Thanks.

mzanm commented 2 years ago

I've made a pr. https://github.com/nvaccess/nvda/pull/13214 What do you all think about this, would a third dropdown list item with restart options better or is the way it is now better? Let me know, Happy to make any changes.

josephsl commented 2 years ago

CC @feerrenrut if you don’t mind looking at GUI side of things please. Thanks.

CyrilleB79 commented 1 month ago

To all participants of this issue:

This issue has not been implemented for 4 years. The discussion regarding an improved UX for the exit dialog has been quite difficult. And when a consensus has been found and implemented (PR #13214), the PR has been blocked by a technical difficulty and has been abandoned. Though, implementing this issue would still be very useful when making support to provide simpler instructions.

I do not wish to spend so much time on this; the better is the enemy of the good.

So I have opened #17043 to address the main concern of this issue, letting the exit dialog UX redesign for a future but less urgent work.