add enable/disable function in addons manager. #3090
Comments
|
Comment 2 by jteh on 2014-09-03 10:13 |
|
Comment 3 by beqa on 2014-09-03 10:26 |
nvaccessAuto
added
enhancement
Addon/management
|
Hi, I think this is an interesting proposal. Is someone working on this or planing to do it? |
|
Hi, once I install VS 2012/2015 (preferably the latter), I’m thinking about designing and working on it (a related ticket should be raised (if not done already) to let users specify which add-ons to disable at startup). Thanks. From: nvdaes [mailto:notifications@github.com] Hi, I think this is an interesting proposal. Is someone working on this or planing to do it? — |
|
OK, then will this ticket be covered? El 28/02/2016 a las 9:00, josephsl escribió:
|
|
Hi, at this point, the first (GUI portion). I think we should ask Zahari and others who’ve done work on #4057 to see if we can extend the command-line option added as part of that ticket. Thanks. From: nvdaes [mailto:notifications@github.com] OK, then will this ticket be covered? El 28/02/2016 a las 9:00, josephsl escribió:
— |
|
OK, then you can work on this ticket, though #4057 is marked as duplicate.
|
|
Hi, Preliminary design: Extend addonHandler.state to record disabled add-ons and add enable/disable button and related routines to present add-on enabled status and to toggle it. Procedure:
Things to discuss:
Currently I'm looking at addonHandler.py and gui/addonGui.py to see what can be done (and update copyright year in the process). Comments on the above procedure are appreciated. Thanks. |
|
Hi, I had thought this:
El 28/02/2016 a las 9:46, josephsl escribió:
|
|
Sorry. El 28/02/2016 a las 10:15, Noelia escribió:
|
|
Hi, For anyone interested in seeing this come to life: in order to support add-on disable/enable UI, a necessary change must be done in add-ons subsystem (addonHandler.py). The posted commit is first of a series of commits that provides foundation for resolving this ticket (see the commit message and the resulting diff for details). My testing shows this commit is working as intended. Also took this time to add a message in log.info (core.py) to let the user know that all add-ons are disabled if this is such a session, and a log.debug message (core.py) to let developers know if the add-on being initialized is in fact a disabled add-on. |
|
Hi all, Once the display string for add-on enable/disable change is agreed upon, I'll commit the next part along with the user guide. Thanks. |
|
Hi, when starting NVDA with these changes in addonHandler and addonGui, CRITICAL - main (05:38:51): I like your approach, and I had more comments about design, but I didn't El 29/02/2016 a las 23:34, josephsl escribió:
|
|
Hi, No opposite of "disableAddonsIfAny": As far as NVDA sees it, add-ons are alive and well except those who are "taking a break" (disabled). KeyError: Fixed in the next commit. Enable/disable status: I think we can borrow what "install" and "remove" status message does (quite a trivial thing to implement, although I need to be careful about which add-ons are truly disabled). I'm thinking status should be updated after user clicks enable/disable (in case of "disable", after confirming it as you pointed out). Speaking of command-line implementation: based on current design, I think we can cover this in i3090 branch, although a separate ticket should be raised that references this ticket (to discuss command line arguments and so on). Also fixed spelling/style mismatch in addonGui event handlers. Thanks (keep your comments coming). |
|
Hi, about not to adding a shortcut key, this can be changed by El 01/03/2016 a las 6:06, josephsl escribió:
|
|
Hi, I think adding a warning before enabling the add-on is quite redundant - add-on installer gives this warning already (unless others believe a warning is needed). Command line/add-on name/index: that's quite tricky. I'm a bit worried about using the command line to install add-ons, as it can install anything given a path to the package, nor removal as users can remove add-ons by accident. Thus I think command line should be reserved for those who know the internal identifier of the add-on (developers or users who are told by add-on authors to do so based on information given). Thanks. |
|
Hi, imo, if an add-on is disabled, makes sense to add a warning when El 01/03/2016 a las 6:32, josephsl escribió:
|
|
Hi, As far as I can tell, it is done, pending code review and integration in various snapshots for testing purposes. as for using a pending enable set: I decided to use the module version, as pendingDisableSet is really intended to let add-ons subsystem know which add-on(s) to disable. This commit also adds strings that'll be shown when you enable or disable add-ons, and the user guide includes a paragraph on add-on enable/disable feature. To whoever will be committing this to next/master: I think squashing it would be better. Thanks (requesting code review at this point before working on command line version). |
|
Hi, I see that add-ons can be enabled/disabled when they are El 01/03/2016 a las 10:46, josephsl escribió:
|
|
I don't think we need different states for suspended and disabled.
Simply don't allow enabling add-ons if all add-ons are disabled via the
command line.
I also don't think we need to allow selective disabling of add-ons via
the command line. If someone runs into trouble, they can just restart
with add-ons disabled temporarily so they can get into the Add-ons
Manager and selectively disable.
|
|
Hi, About the command line - the user will probably need the internal add-on name. So if you decide to add this option, consider an option for listing all add-ons including their internal name and eventually showing their state. If the change is permanent, it is better if we have a separate option for disabling, as well as enabling an addon, instead of reusing --disable-addons, which is meant to disable all add-ons temporary.. Those options might accept a comma separated list of internal add-on names. So that counts three - --enable-addon=comma_separated_list, --disable-addon=comma_separated_list and --list-addons. |
|
Hi, thanks for all your comments. El 02/03/2016 a las 0:14, Zahari Yurukov escribió:
|
|
Hi all, After thinking about command line options and reading various comments, I'd like to request that we don't implement selective add-on suspension via command line. Reasons are thus:
As for "suspended" versus "disabled": If I heard @jcsteh right, he prefers current strings (I agree with this). But as for selectively disabling an add-on while all add-ons are disabled: not sure, but I'm leaning towards not doing that, as one of the reasons for running NVDA Core in "safe mode" - no add-ons - is to serve as a starting point for troubleshooting issues that may or may not involve add-ons. If an issue is certainly with an add-on (or determined as such), users can restart NVDA in normal mode and selectively disable it from add-ons manager (I think the title of this dialog should reflect the fact that all add-ons are disabled, not just description text, which I think should be a separate ticket and is trivial to implement). Another option is to add a checkbox that only shows up when NVDA is running in safe mode that adds the offending add-on to "disabled" list even when NVDA runs in normal mode until enabled. Comments are appreciated. Thanks. |
|
IMO, doing this kind of thing from the command line is totally
unnecessary and makes things overly complex. The path to hell is paved
with over-engineering.
|
|
Hi, I agree with Jamie at this point: I think that it's better to allow El 02/03/2016 a las 7:49, josephsl escribió:
|
|
Hi, A checkbox labeled "keep this add-on disabled" would be a good idea (@jcsteh, is this fine with you and other devs and users?). Once I manage to compile latest master with VS2015 Express for Desktop (C++ isn't included by default), I'll work on this suggestion and modify user guide accordingly (and removing reference to command line work). Thanks. |
|
Hi, when all add-ons are disabled, and also an add-on was already El 02/03/2016 a las 9:05, josephsl escribió:
|
|
Hi, latest commit to i3090 provided a fix for it (the commit before merging master/VC2015). From: nvdaes [mailto:notifications@github.com] Hi, when all add-ons are disabled, and also an add-on was already El 02/03/2016 a las 9:05, josephsl escribió:
— |
…, as it is not feasible. re nvaccess#3090
…rt screen. re nvaccess#3090 Good catch by Derek Riemer: always strive for consistency in user interface strings.
…ling add-ons, simplify restart prompt. re nvaccess#3090. Code review from Mick Curran (NV Access): * Restart prompt: quite complicated (agreed). Thus simplify to say that changes were made to add-ons. * Disable warning: Although it is good to give users warnings similar to the one shown when installing an add-on, it would be better to just disable or enable it and be done with it. Thus no warning from now on. * User guide: linguistic fixes.
…t a restart. re nvaccess#3090 Consider the following scenario: user opens add-ons manager, disables and add-on, then enables it without a restart. Status columns should say 'running' as opposed to 'enable' (the current one is redundant, as enabling means running). Caught by Mick Curran (NV Access): NvDA will just say 'runing' for this case.
Caught by Mick Curran: testing shows second check of addon.pendingDisable is actually unnecessary (besides being a performance issue later), hence pendingDisable set wilol be consulted only once.
… for cardinality of sets and comparing sets. re nvaccess#3090. As noted by Michael Curran: it is better to make needsRestart a property. However given that it is consulted only when closing, it is better to refactor by: * Check if any add-ons have been installed or removed ( tehnically, pending). * Check if disabled add-ons and pending disable set are the same (for the most part, it is). This also fixes an issue where if restart dialog is dismissed when closing add-ons manager, and add-ons manager is opened again, restart prompt would not be displayed.
…ler.terminate, rewrote needsRestart. re nvaccess#3090 Comments from Tyler Spivey and Michael Curran: when disabled, an add-on should stay disabled until enabled later. To hold this informaiton, addonHandler.termiante is now fleshed out with various set operations (intersection nad set difference mostly) to let NVDA know which add-ons should stay deactivated. Also rewrote needsRestart so it'll go through add-ons and raise the flag if the following conditions are seen: pending install, pending remove, about to come back to life (enabled), and about to stay ou4t of business (disabled).
…hat we have a persistent backup in case of disasters such as crashes. re nvaccess#3090. A useful that wasn't thought of before (outlined by Michael Curran): what if NVDA suddenly crashes? If this happens, the previous states map would not record add-ons that are indeed disabled because pendingDisableSet is gone. Thus two new sets are introduced: * DisabledAddons: records add-ons that are actually disabled. * pendingEnableSet: Tells NVDA to enable the listed add-ons next time it is run. Due to this change: * pendingDisableSet will no longer record add-ons that'll be disabled during the next session. This set will now hold a list of add-ons that'll be turned off next time it loads. * Runtime counterparts of these sets will be kept in memory. When a crash occurs, persistent copy will be loaded.
…ntime states denoting future enable/disable. re nvaccess#3090
…s pending enable. re nvaccess#3090
…ed and is pending enable. re nvaccess#3090" This reverts commit f4e3852.
|
Merged in d9b3ac1. |
|
Hi, Shouldn't it be: I.e. isn't it really disabled only when it's... disabled? In any other case shouldn't it be considered enabled, as far as it concerns the case-per-case enabling/disabling, regardless the global disable flag? In my opinion - yes. |
|
An add-on is marked as suspended when global disable is set, regardless of whether it is enabled or disabled. So, if the add-on was disabled, you can certainly enable while it's suspended. If an add-on is scheduled to be disabled on the next restart, it is marked as "disable", but you can undo this by enabling. There's actually no "disabled" state. The documentation is wrong regarding several things here. I think I caused a bit of confusion with some of my earlier thoughts. I'm not really a fan of the "suspended" term; users are going to think of it as "disabled" even with the global flag because it says "Restart with add-ons disabled". That said, I understand that users might use the state to determine whether they can explicitly enable or disable the add-on. Regardless, it seems we definitely need a bit of tweaking here, but it's too late to do this for 2016.3. I'll file a follow up issue. |
|
On further thought, perhaps we should leave "suspended" as is. The problem with renaming it to "disabled" is that for global disable, pressing the button will make the state change from "disabled" to "disable", which is even more confusing. Ug. At the very least, we definitely need to clarify the documentation for 2016.4. I wonder whether we should just completely remove the sentence @zahyur was querying. Whether the add-on can be enabled or disabled is enforced by the GUI anyway. If the enable button is present, the add-on can be enabled. If the disable button is present, the add-on can be disabled. It's worth noting that Firefox just says "disabled" for both global and explicit disabling. |
|
Hi, will wait for a follow-up issue before posting a possible PR. Thanks. From: James Teh [mailto:notifications@github.com] On further thought, perhaps we should leave "suspended" as is. The problem with renaming it to "disabled" is that for global disable, pressing the button will make the state change from "disabled" to "disable", which is even more confusing. Ug. At the very least, we definitely need to clarify the documentation for 2016.4. I wonder whether we should just completely remove the sentence @zahyur https://github.com/zahyur was querying. Whether the add-on can be enabled or disabled is enforced by the GUI anyway. If the enable button is present, the add-on can be enabled. If the disable button is present, the add-on can be disabled. It's worth noting that Firefox just says "disabled" for both global and explicit disabling. — |
|
Hi, I.e. in Firefox, it's probably absolutely the same, except the disabled state isn't saved for individual add-ons, if it's do to a global switch. They use the same word, cause it's practically the same - you can't disable already disabled add-on.Best wishes, |
|
On the contrary, if you disable add-ons globally (which requires a
restart), they show up as disabled, but you can still hit Disable to
explicitly disable an add-on. If an add-on was already disabled, you can
hit Enable to re-enable it.
|
Reported by beqa on 2013-03-17 13:01
hi devs.
what are you thinking about adding enable/disable function in addons manager?
we will be able to disable/enable an addon not delete.
if you will find this interesting i am ready to work on this.
thanks
Blocking #4437
The text was updated successfully, but these errors were encountered: