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

add enable/disable function in addons manager. #3090

Closed
nvaccessAuto opened this Issue Mar 17, 2013 · 39 comments

Comments

Projects
None yet
6 participants
@nvaccessAuto

nvaccessAuto commented Mar 17, 2013

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

@nvaccessAuto

This comment has been minimized.

Show comment
Hide comment
@nvaccessAuto

nvaccessAuto Sep 3, 2014

Comment 2 by jteh on 2014-09-03 10:13
Beqa, are you still interested in working on this? Note that you'd still need to restart NVDA for this to take effect; there are simply too many unknowns otherwise. However, this is probably still better than, for example, removing an add-on just to test if something works without it running.

nvaccessAuto commented Sep 3, 2014

Comment 2 by jteh on 2014-09-03 10:13
Beqa, are you still interested in working on this? Note that you'd still need to restart NVDA for this to take effect; there are simply too many unknowns otherwise. However, this is probably still better than, for example, removing an add-on just to test if something works without it running.

@nvaccessAuto

This comment has been minimized.

Show comment
Hide comment
@nvaccessAuto

nvaccessAuto Sep 3, 2014

Comment 3 by beqa on 2014-09-03 10:26
yes, in nar future i will start to work on this.

nvaccessAuto commented Sep 3, 2014

Comment 3 by beqa on 2014-09-03 10:26
yes, in nar future i will start to work on this.

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Feb 28, 2016

Contributor

Hi, I think this is an interesting proposal. Is someone working on this or planing to do it?
If not, I can try to create a pull request
Thanks.

Contributor

nvdaes commented Feb 28, 2016

Hi, I think this is an interesting proposal. Is someone working on this or planing to do it?
If not, I can try to create a pull request
Thanks.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Feb 28, 2016

Collaborator

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]
Sent: Saturday, February 27, 2016 11:59 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

Hi, I think this is an interesting proposal. Is someone working on this or planing to do it?
If not, I can try to create a pull request
Thanks.


Reply to this email directly or view it on GitHub #3090 (comment) .

Collaborator

josephsl commented Feb 28, 2016

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]
Sent: Saturday, February 27, 2016 11:59 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

Hi, I think this is an interesting proposal. Is someone working on this or planing to do it?
If not, I can try to create a pull request
Thanks.


Reply to this email directly or view it on GitHub #3090 (comment) .

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Feb 28, 2016

Contributor

OK, then will this ticket be covered?
This is for enabling add-ons too.
I don't know if you are planing to work on command line options, in
add-on manager adding enable/disable buttons, or both.
I was thinking just in the seccond one.
Cheers.

El 28/02/2016 a las 9:00, josephsl escribió:

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]
Sent: Saturday, February 27, 2016 11:59 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

Hi, I think this is an interesting proposal. Is someone working on
this or planing to do it?
If not, I can try to create a pull request
Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment) .


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Feb 28, 2016

OK, then will this ticket be covered?
This is for enabling add-ons too.
I don't know if you are planing to work on command line options, in
add-on manager adding enable/disable buttons, or both.
I was thinking just in the seccond one.
Cheers.

El 28/02/2016 a las 9:00, josephsl escribió:

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]
Sent: Saturday, February 27, 2016 11:59 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

Hi, I think this is an interesting proposal. Is someone working on
this or planing to do it?
If not, I can try to create a pull request
Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment) .


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Feb 28, 2016

Collaborator

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]
Sent: Sunday, February 28, 2016 12:05 AM
To: nvaccess/nvda nvda@noreply.github.com
Cc: josephsl joseph.lee22590@gmail.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

OK, then will this ticket be covered?
This is for enabling add-ons too.
I don't know if you are planing to work on command line options, in
add-on manager adding enable/disable buttons, or both.
I was thinking just in the seccond one.
Cheers.

El 28/02/2016 a las 9:00, josephsl escribió:

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]
Sent: Saturday, February 27, 2016 11:59 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

Hi, I think this is an interesting proposal. Is someone working on
this or planing to do it?
If not, I can try to create a pull request
Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment) .


Reply to this email directly or view it on GitHub
#3090 (comment).


Reply to this email directly or view it on GitHub #3090 (comment) .

Collaborator

josephsl commented Feb 28, 2016

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]
Sent: Sunday, February 28, 2016 12:05 AM
To: nvaccess/nvda nvda@noreply.github.com
Cc: josephsl joseph.lee22590@gmail.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

OK, then will this ticket be covered?
This is for enabling add-ons too.
I don't know if you are planing to work on command line options, in
add-on manager adding enable/disable buttons, or both.
I was thinking just in the seccond one.
Cheers.

El 28/02/2016 a las 9:00, josephsl escribió:

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]
Sent: Saturday, February 27, 2016 11:59 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

Hi, I think this is an interesting proposal. Is someone working on
this or planing to do it?
If not, I can try to create a pull request
Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment) .


Reply to this email directly or view it on GitHub
#3090 (comment).


Reply to this email directly or view it on GitHub #3090 (comment) .

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Feb 28, 2016

Contributor

OK, then you can work on this ticket, though #4057 is marked as duplicate.
Hope this can be added in the next release.
Thanks.
El 28/02/2016 a las 9:08, 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]
Sent: Sunday, February 28, 2016 12:05 AM
To: nvaccess/nvda nvda@noreply.github.com
Cc: josephsl joseph.lee22590@gmail.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

OK, then will this ticket be covered?
This is for enabling add-ons too.
I don't know if you are planing to work on command line options, in
add-on manager adding enable/disable buttons, or both.
I was thinking just in the seccond one.
Cheers.

El 28/02/2016 a las 9:00, josephsl escribió:

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]
Sent: Saturday, February 27, 2016 11:59 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] add enable/disable function in addons manager.
(#3090)

Hi, I think this is an interesting proposal. Is someone working on
this or planing to do it?
If not, I can try to create a pull request
Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment) .


Reply to this email directly or view it on GitHub
#3090 (comment).


Reply to this email directly or view it on GitHub
#3090 (comment) .


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Feb 28, 2016

OK, then you can work on this ticket, though #4057 is marked as duplicate.
Hope this can be added in the next release.
Thanks.
El 28/02/2016 a las 9:08, 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]
Sent: Sunday, February 28, 2016 12:05 AM
To: nvaccess/nvda nvda@noreply.github.com
Cc: josephsl joseph.lee22590@gmail.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

OK, then will this ticket be covered?
This is for enabling add-ons too.
I don't know if you are planing to work on command line options, in
add-on manager adding enable/disable buttons, or both.
I was thinking just in the seccond one.
Cheers.

El 28/02/2016 a las 9:00, josephsl escribió:

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]
Sent: Saturday, February 27, 2016 11:59 PM
To: nvaccess/nvda nvda@noreply.github.com
Subject: Re: [nvda] add enable/disable function in addons manager.
(#3090)

Hi, I think this is an interesting proposal. Is someone working on
this or planing to do it?
If not, I can try to create a pull request
Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment) .


Reply to this email directly or view it on GitHub
#3090 (comment).


Reply to this email directly or view it on GitHub
#3090 (comment) .


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Feb 28, 2016

Collaborator

Hi,
I see that Beqa wanted to work on this - are you interested in working on this? If so, let us know.

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:

  1. User opens add-ons manager.
  2. User selects an entry that should be disabled (clicks "disable" button). The add-ons manager user interface:
    1. Checks if the selected add-on (internal name) is part of "disabled add-ons" set in addonHandler.state, and if so, display "enable", "disable" otherwise.
    2. If "disable" is shown, once the button is clicked, add the add-on entry into the disabled list.
    3. If "enable" is shown and if record indicates the add-on is disabled, remove it from the state set.
  3. User restarts NVDA. The add-on loader routine in addonHandler:
    1. For each add-on, scans addonHandler.state to see if it should be disabled.
    2. If the cardinality of the disabled add-ons set matches number of "valid" add-ons loaded (just in case the add-on is no longer installed), apply #4057 routine (all add-ons are suspended). Also leave room for future extensions such as receiving list of add-ons to be disabled from the command line.

Things to discuss:

  • When displaying add-on status, is "running" and "suspended" enough? I feel we need to add "suspend pending" or "running pending".
  • After enabling/disabling one or more add-ons, should the user be told to restart NVDA when closing add-ons manager?
  • What if a suspended add-on is removed, not using add-ons manager but by simply deleting the add-on directory tree? One way to ensure this doesn't happen is checking to really make sure the disabled add-ons set matches installed add-ons when NVDA restarts, and if not, flush those that are not found.

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.

Collaborator

josephsl commented Feb 28, 2016

Hi,
I see that Beqa wanted to work on this - are you interested in working on this? If so, let us know.

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:

  1. User opens add-ons manager.
  2. User selects an entry that should be disabled (clicks "disable" button). The add-ons manager user interface:
    1. Checks if the selected add-on (internal name) is part of "disabled add-ons" set in addonHandler.state, and if so, display "enable", "disable" otherwise.
    2. If "disable" is shown, once the button is clicked, add the add-on entry into the disabled list.
    3. If "enable" is shown and if record indicates the add-on is disabled, remove it from the state set.
  3. User restarts NVDA. The add-on loader routine in addonHandler:
    1. For each add-on, scans addonHandler.state to see if it should be disabled.
    2. If the cardinality of the disabled add-ons set matches number of "valid" add-ons loaded (just in case the add-on is no longer installed), apply #4057 routine (all add-ons are suspended). Also leave room for future extensions such as receiving list of add-ons to be disabled from the command line.

Things to discuss:

  • When displaying add-on status, is "running" and "suspended" enough? I feel we need to add "suspend pending" or "running pending".
  • After enabling/disabling one or more add-ons, should the user be told to restart NVDA when closing add-ons manager?
  • What if a suspended add-on is removed, not using add-ons manager but by simply deleting the add-on directory tree? One way to ensure this doesn't happen is checking to really make sure the disabled add-ons set matches installed add-ons when NVDA restarts, and if not, flush those that are not found.

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.

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Feb 28, 2016

Contributor

Hi, I had thought this:

  1. In addonHandler:
    a. Add a pendingDisable state, similar to pendingInstall, and a disabled
    state too.
    b. Add a requestDisable function, as requestRemove(), which marks as
    disabled the add-ons marked as pendingDisable, which would be added to
    initialize function.
    c. In isRunning functionproperty, return True if the add-on is not in
    pendingInstall or disabled states.
  2. In addonGui:
    a. Disable enable/disable buttons, and Enable them when:
  • Add-ons are not suspended due to restart with disable add-ons
    function, and when the current add-on is not marked as pendingInstall or
    pendingRemove.
    c. Disable the Remove button for add-ons marked as pendingDisable, since
    it could be redundant and because disable an add-on is intended
    precissely to test without not delete add-ons.
    This buttons (enable/disable) could be placed after the About button,
    before Install and Remove, since Remove is a drastic method and disable
    can be preferred to test. and presented as a first option.
    Then, the possible states shown for add-on could be:
    a. Install (pendingInstall).
    b. Remove (pendingRemove).
    c. Disable (pendingDisable).
    d. Disabled (when an add-on has been marked as pendingDisable and NVDA
    has been restarted), and then the state turn into Disabled.
    e. Running otherwise.
    Also, I think that a question to confirm is an add-on is going to be
    enabled or disabled should be presented.
    Furthermore, the message about that add-ons are been installed or
    removed could be changed: something like: "Status of add-ons has
    changed. Do you want to restart NVDA...?.
    Cheers.

El 28/02/2016 a las 9:46, josephsl escribió:

Hi,
I see that Beqa wanted to work on this - are you interested in working
on this? If so, let us know.

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:

  1. User opens add-ons manager.
  2. User selects an entry that should be disabled (clicks "disable"
    button). The add-ons manager user interface:
    1. Checks if the selected add-on (internal name) is part of
      "disabled add-ons" set in addonHandler.state, and if so,
      display "enable", "disable" otherwise.
    2. If "disable" is shown, once the button is clicked, add the
      add-on entry into the disabled list.
    3. If "enable" is shown and if record indicates the add-on is
      disabled, remove it from the state set.
  3. User restarts NVDA. The add-on loader routine in addonHandler:
    1. For each add-on, scans addonHandler.state to see if it should
      be disabled.
    2. If the cardinality of the disabled add-ons set matches number
      of "valid" add-ons loaded (just in case the add-on is no
      longer installed), apply #4057
      #4057 routine (all
      add-ons are suspended). Also leave room for future extensions
      such as receiving list of add-ons to be disabled from the
      command line.

Things to discuss:

  • When displaying add-on status, is "running" and "suspended"
    enough? I feel we need to add "suspend pending" or "running pending".
  • After enabling/disabling one or more add-ons, should the user be
    told to restart NVDA when closing add-ons manager?
  • What if a suspended add-on is removed, not using add-ons manager
    but by simply deleting the add-on directory tree? One way to
    ensure this doesn't happen is checking to really make sure the
    disabled add-ons set matches installed add-ons when NVDA restarts,
    and if not, flush those that are not found.

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.


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Feb 28, 2016

Hi, I had thought this:

  1. In addonHandler:
    a. Add a pendingDisable state, similar to pendingInstall, and a disabled
    state too.
    b. Add a requestDisable function, as requestRemove(), which marks as
    disabled the add-ons marked as pendingDisable, which would be added to
    initialize function.
    c. In isRunning functionproperty, return True if the add-on is not in
    pendingInstall or disabled states.
  2. In addonGui:
    a. Disable enable/disable buttons, and Enable them when:
  • Add-ons are not suspended due to restart with disable add-ons
    function, and when the current add-on is not marked as pendingInstall or
    pendingRemove.
    c. Disable the Remove button for add-ons marked as pendingDisable, since
    it could be redundant and because disable an add-on is intended
    precissely to test without not delete add-ons.
    This buttons (enable/disable) could be placed after the About button,
    before Install and Remove, since Remove is a drastic method and disable
    can be preferred to test. and presented as a first option.
    Then, the possible states shown for add-on could be:
    a. Install (pendingInstall).
    b. Remove (pendingRemove).
    c. Disable (pendingDisable).
    d. Disabled (when an add-on has been marked as pendingDisable and NVDA
    has been restarted), and then the state turn into Disabled.
    e. Running otherwise.
    Also, I think that a question to confirm is an add-on is going to be
    enabled or disabled should be presented.
    Furthermore, the message about that add-ons are been installed or
    removed could be changed: something like: "Status of add-ons has
    changed. Do you want to restart NVDA...?.
    Cheers.

El 28/02/2016 a las 9:46, josephsl escribió:

Hi,
I see that Beqa wanted to work on this - are you interested in working
on this? If so, let us know.

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:

  1. User opens add-ons manager.
  2. User selects an entry that should be disabled (clicks "disable"
    button). The add-ons manager user interface:
    1. Checks if the selected add-on (internal name) is part of
      "disabled add-ons" set in addonHandler.state, and if so,
      display "enable", "disable" otherwise.
    2. If "disable" is shown, once the button is clicked, add the
      add-on entry into the disabled list.
    3. If "enable" is shown and if record indicates the add-on is
      disabled, remove it from the state set.
  3. User restarts NVDA. The add-on loader routine in addonHandler:
    1. For each add-on, scans addonHandler.state to see if it should
      be disabled.
    2. If the cardinality of the disabled add-ons set matches number
      of "valid" add-ons loaded (just in case the add-on is no
      longer installed), apply #4057
      #4057 routine (all
      add-ons are suspended). Also leave room for future extensions
      such as receiving list of add-ons to be disabled from the
      command line.

Things to discuss:

  • When displaying add-on status, is "running" and "suspended"
    enough? I feel we need to add "suspend pending" or "running pending".
  • After enabling/disabling one or more add-ons, should the user be
    told to restart NVDA when closing add-ons manager?
  • What if a suspended add-on is removed, not using add-ons manager
    but by simply deleting the add-on directory tree? One way to
    ensure this doesn't happen is checking to really make sure the
    disabled add-ons set matches installed add-ons when NVDA restarts,
    and if not, flush those that are not found.

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.


Reply to this email directly or view it on GitHub
#3090 (comment).

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Feb 28, 2016

Contributor

Sorry.
About Enable butto, I think it should be available for add-ons marked as
Dissabled or pendingDisable, to prevent mistakes.
Otherwise, disable button should be available for running add-ons not
marked as pendingDisable.
When an add-on is to be removed (in pendingRemove state), I think these
buttons shouldn't be shown.
Thanks.

El 28/02/2016 a las 10:15, Noelia escribió:

Hi, I had thought this:

  1. In addonHandler:
    a. Add a pendingDisable state, similar to pendingInstall, and a
    disabled state too.
    b. Add a requestDisable function, as requestRemove(), which marks as
    disabled the add-ons marked as pendingDisable, which would be added to
    initialize function.
    c. In isRunning functionproperty, return True if the add-on is not in
    pendingInstall or disabled states.
  2. In addonGui:
    a. Disable enable/disable buttons, and Enable them when:
  • Add-ons are not suspended due to restart with disable add-ons
    function, and when the current add-on is not marked as pendingInstall
    or pendingRemove.
    c. Disable the Remove button for add-ons marked as pendingDisable,
    since it could be redundant and because disable an add-on is intended
    precissely to test without not delete add-ons.
    This buttons (enable/disable) could be placed after the About button,
    before Install and Remove, since Remove is a drastic method and
    disable can be preferred to test. and presented as a first option.
    Then, the possible states shown for add-on could be:
    a. Install (pendingInstall).
    b. Remove (pendingRemove).
    c. Disable (pendingDisable).
    d. Disabled (when an add-on has been marked as pendingDisable and NVDA
    has been restarted), and then the state turn into Disabled.
    e. Running otherwise.
    Also, I think that a question to confirm is an add-on is going to be
    enabled or disabled should be presented.
    Furthermore, the message about that add-ons are been installed or
    removed could be changed: something like: "Status of add-ons has
    changed. Do you want to restart NVDA...?.
    Cheers.

El 28/02/2016 a las 9:46, josephsl escribió:

Hi,
I see that Beqa wanted to work on this - are you interested in
working on this? If so, let us know.

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:

  1. User opens add-ons manager.
  2. User selects an entry that should be disabled (clicks "disable"
    button). The add-ons manager user interface:
    1. Checks if the selected add-on (internal name) is part of
      "disabled add-ons" set in addonHandler.state, and if so,
      display "enable", "disable" otherwise.
    2. If "disable" is shown, once the button is clicked, add the
      add-on entry into the disabled list.
    3. If "enable" is shown and if record indicates the add-on is
      disabled, remove it from the state set.
  3. User restarts NVDA. The add-on loader routine in addonHandler:
    1. For each add-on, scans addonHandler.state to see if it should
      be disabled.
    2. If the cardinality of the disabled add-ons set matches number
      of "valid" add-ons loaded (just in case the add-on is no
      longer installed), apply #4057
      #4057 routine (all
      add-ons are suspended). Also leave room for future extensions
      such as receiving list of add-ons to be disabled from the
      command line.

Things to discuss:

  • When displaying add-on status, is "running" and "suspended"
    enough? I feel we need to add "suspend pending" or "running pending".
  • After enabling/disabling one or more add-ons, should the user be
    told to restart NVDA when closing add-ons manager?
  • What if a suspended add-on is removed, not using add-ons manager
    but by simply deleting the add-on directory tree? One way to
    ensure this doesn't happen is checking to really make sure the
    disabled add-ons set matches installed add-ons when NVDA
    restarts, and if not, flush those that are not found.

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.


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Feb 28, 2016

Sorry.
About Enable butto, I think it should be available for add-ons marked as
Dissabled or pendingDisable, to prevent mistakes.
Otherwise, disable button should be available for running add-ons not
marked as pendingDisable.
When an add-on is to be removed (in pendingRemove state), I think these
buttons shouldn't be shown.
Thanks.

El 28/02/2016 a las 10:15, Noelia escribió:

Hi, I had thought this:

  1. In addonHandler:
    a. Add a pendingDisable state, similar to pendingInstall, and a
    disabled state too.
    b. Add a requestDisable function, as requestRemove(), which marks as
    disabled the add-ons marked as pendingDisable, which would be added to
    initialize function.
    c. In isRunning functionproperty, return True if the add-on is not in
    pendingInstall or disabled states.
  2. In addonGui:
    a. Disable enable/disable buttons, and Enable them when:
  • Add-ons are not suspended due to restart with disable add-ons
    function, and when the current add-on is not marked as pendingInstall
    or pendingRemove.
    c. Disable the Remove button for add-ons marked as pendingDisable,
    since it could be redundant and because disable an add-on is intended
    precissely to test without not delete add-ons.
    This buttons (enable/disable) could be placed after the About button,
    before Install and Remove, since Remove is a drastic method and
    disable can be preferred to test. and presented as a first option.
    Then, the possible states shown for add-on could be:
    a. Install (pendingInstall).
    b. Remove (pendingRemove).
    c. Disable (pendingDisable).
    d. Disabled (when an add-on has been marked as pendingDisable and NVDA
    has been restarted), and then the state turn into Disabled.
    e. Running otherwise.
    Also, I think that a question to confirm is an add-on is going to be
    enabled or disabled should be presented.
    Furthermore, the message about that add-ons are been installed or
    removed could be changed: something like: "Status of add-ons has
    changed. Do you want to restart NVDA...?.
    Cheers.

El 28/02/2016 a las 9:46, josephsl escribió:

Hi,
I see that Beqa wanted to work on this - are you interested in
working on this? If so, let us know.

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:

  1. User opens add-ons manager.
  2. User selects an entry that should be disabled (clicks "disable"
    button). The add-ons manager user interface:
    1. Checks if the selected add-on (internal name) is part of
      "disabled add-ons" set in addonHandler.state, and if so,
      display "enable", "disable" otherwise.
    2. If "disable" is shown, once the button is clicked, add the
      add-on entry into the disabled list.
    3. If "enable" is shown and if record indicates the add-on is
      disabled, remove it from the state set.
  3. User restarts NVDA. The add-on loader routine in addonHandler:
    1. For each add-on, scans addonHandler.state to see if it should
      be disabled.
    2. If the cardinality of the disabled add-ons set matches number
      of "valid" add-ons loaded (just in case the add-on is no
      longer installed), apply #4057
      #4057 routine (all
      add-ons are suspended). Also leave room for future extensions
      such as receiving list of add-ons to be disabled from the
      command line.

Things to discuss:

  • When displaying add-on status, is "running" and "suspended"
    enough? I feel we need to add "suspend pending" or "running pending".
  • After enabling/disabling one or more add-ons, should the user be
    told to restart NVDA when closing add-ons manager?
  • What if a suspended add-on is removed, not using add-ons manager
    but by simply deleting the add-on directory tree? One way to
    ensure this doesn't happen is checking to really make sure the
    disabled add-ons set matches installed add-ons when NVDA
    restarts, and if not, flush those that are not found.

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.


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Feb 29, 2016

Collaborator

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.
More commits to come soon. Thanks.

Collaborator

josephsl commented Feb 29, 2016

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.
More commits to come soon. Thanks.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 1, 2016

Collaborator

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.

Collaborator

josephsl commented Mar 1, 2016

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.

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Mar 1, 2016

Contributor

Hi, when starting NVDA with these changes in addonHandler and addonGui,
I get this error


CRITICAL - main (05:38:51):
core failure
Traceback (most recent call last):
File "nvda.pyw", line 190, in
core.main()
File "core.py", line 189, in main
addonHandler.initialize()
File "addonHandler.py", line 116, in initialize
disableAddonsIfAny()
File "addonHandler.py", line 107, in disableAddonsIfAny
_disabledAddons = state["pendingDisableSet"]
KeyError: 'pendingDisableSet'


I like your approach, and I had more comments about design, but I didn't
comment anymore to not clutter the issue.
Anyway, I don't see a function to enableAddonsIfAny, just to disable.
I though about the toggle button, which would add or discard the add-ons
marked as pendingEnableOrDisable (in this state) to the disabledAddons
set, found in the pickle file.
Also I thought that perhaps it will be better, as done with remove
button, to present a confirmation message before adding an addon to the
enableOrDisableSet.
In this way, add-ons in the pendingEnableOrDisableSet, not disabled now,
could show a disable state, disabled add-ons in the
pendingEnableOrDisableSet show an enable state, and disabled add-ons not
in the pendingsDisable, just disabled.
So, the toggle buton just should add the add-on to the pending state,
regardless of its label.
Instead of disableAddonsIfAny perhaps your method can be named something
like completeTogleEnable or omething.
Thanks for this important work.

El 29/02/2016 a las 23:34, josephsl 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.
More commits to come soon. Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Mar 1, 2016

Hi, when starting NVDA with these changes in addonHandler and addonGui,
I get this error


CRITICAL - main (05:38:51):
core failure
Traceback (most recent call last):
File "nvda.pyw", line 190, in
core.main()
File "core.py", line 189, in main
addonHandler.initialize()
File "addonHandler.py", line 116, in initialize
disableAddonsIfAny()
File "addonHandler.py", line 107, in disableAddonsIfAny
_disabledAddons = state["pendingDisableSet"]
KeyError: 'pendingDisableSet'


I like your approach, and I had more comments about design, but I didn't
comment anymore to not clutter the issue.
Anyway, I don't see a function to enableAddonsIfAny, just to disable.
I though about the toggle button, which would add or discard the add-ons
marked as pendingEnableOrDisable (in this state) to the disabledAddons
set, found in the pickle file.
Also I thought that perhaps it will be better, as done with remove
button, to present a confirmation message before adding an addon to the
enableOrDisableSet.
In this way, add-ons in the pendingEnableOrDisableSet, not disabled now,
could show a disable state, disabled add-ons in the
pendingEnableOrDisableSet show an enable state, and disabled add-ons not
in the pendingsDisable, just disabled.
So, the toggle buton just should add the add-on to the pending state,
regardless of its label.
Instead of disableAddonsIfAny perhaps your method can be named something
like completeTogleEnable or omething.
Thanks for this important work.

El 29/02/2016 a las 23:34, josephsl 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.
More commits to come soon. Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 1, 2016

Collaborator

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).
As for no confirmation dialog when add-ons are enabled/disabled: I'd imagine users would just click the button and let NVDA do the right thing (no hotkeys assigned to prevent accidental clicking, as this button must be clicked by those who know what they are doing). Perhaps we should add a warning text (part of add-ons manager) saying one or more add-ons are disabled if this is the case (the current text shown when all add-ons are enabled isn't quite right (grammatically)). But adding a confirmation dialog when disabling add-ons is a good point (at least warn users that they need to do this if they know what they are doing).

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

Collaborator

josephsl commented Mar 1, 2016

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).
As for no confirmation dialog when add-ons are enabled/disabled: I'd imagine users would just click the button and let NVDA do the right thing (no hotkeys assigned to prevent accidental clicking, as this button must be clicked by those who know what they are doing). Perhaps we should add a warning text (part of add-ons manager) saying one or more add-ons are disabled if this is the case (the current text shown when all add-ons are enabled isn't quite right (grammatically)). But adding a confirmation dialog when disabling add-ons is a good point (at least warn users that they need to do this if they know what they are doing).

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

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Mar 1, 2016

Contributor

Hi, about not to adding a shortcut key, this can be changed by
translators and, at least in Spanish, all the buttons has a shortcut key
in add-ons manager, including remove and install.
I think that enable add-ons can be as bad as disabling them, mostly in
case of add-ons shared in non trust sites.
Abou the command line, I think it can be tricky to implement since the
user needs to know the index or name of the add-on which want to enable
or disable. Furthermore, it can be extended not just to enable/disable,
but to install or remove add-ons.
Cheers.

El 01/03/2016 a las 6:06, 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).
As for no confirmation dialog when add-ons are enabled/disabled: I'd
imagine users would just click the button and let NVDA do the right
thing (no hotkeys assigned to prevent accidental clicking, as this
button must be clicked by those who know what they are doing). Perhaps
we should add a warning text (part of add-ons manager) saying one or
more add-ons are disabled if this is the case (the current text shown
when all add-ons are enabled isn't quite right (grammatically)). But
adding a confirmation dialog when disabling add-ons is a good point
(at least warn users that they need to do this if they know what they
are doing).

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


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Mar 1, 2016

Hi, about not to adding a shortcut key, this can be changed by
translators and, at least in Spanish, all the buttons has a shortcut key
in add-ons manager, including remove and install.
I think that enable add-ons can be as bad as disabling them, mostly in
case of add-ons shared in non trust sites.
Abou the command line, I think it can be tricky to implement since the
user needs to know the index or name of the add-on which want to enable
or disable. Furthermore, it can be extended not just to enable/disable,
but to install or remove add-ons.
Cheers.

El 01/03/2016 a las 6:06, 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).
As for no confirmation dialog when add-ons are enabled/disabled: I'd
imagine users would just click the button and let NVDA do the right
thing (no hotkeys assigned to prevent accidental clicking, as this
button must be clicked by those who know what they are doing). Perhaps
we should add a warning text (part of add-ons manager) saying one or
more add-ons are disabled if this is the case (the current text shown
when all add-ons are enabled isn't quite right (grammatically)). But
adding a confirmation dialog when disabling add-ons is a good point
(at least warn users that they need to do this if they know what they
are doing).

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


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 1, 2016

Collaborator

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.

Collaborator

josephsl commented Mar 1, 2016

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.

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Mar 1, 2016

Contributor

Hi, imo, if an add-on is disabled, makes sense to add a warning when
enabled, since perhaps the user has disabled it to prevent conflicts
with other add-on or something.
Just an opinion.
Thanks.

El 01/03/2016 a las 6:32, 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.


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Mar 1, 2016

Hi, imo, if an add-on is disabled, makes sense to add a warning when
enabled, since perhaps the user has disabled it to prevent conflicts
with other add-on or something.
Just an opinion.
Thanks.

El 01/03/2016 a las 6:32, 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.


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 1, 2016

Collaborator

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

Collaborator

josephsl commented Mar 1, 2016

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

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Mar 1, 2016

Contributor

Hi, I see that add-ons can be enabled/disabled when they are
pendingInstall state, but maybe that the installation isn't successful,
i.e. if the onInstall function of installTask can't be run correctly.
Furthermore, perhaps it would be good to distinghish suspended and
disabled add-ons:
Suspended is something temporary, due to global vars, and the add-ons
manager doesn't have the option to restart NVDA with suspended add-ons.
So it doesn't make sense to enable an add-on shown as suspended if it is
already enabled.
If you prefer, I could try to do a pull request on your branch based on
your work, since I don't have too much time, at least tomorrow. Or maybe
another people can continue testing this interesting feature.
Thanks.

El 01/03/2016 a las 10:46, 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).


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Mar 1, 2016

Hi, I see that add-ons can be enabled/disabled when they are
pendingInstall state, but maybe that the installation isn't successful,
i.e. if the onInstall function of installTask can't be run correctly.
Furthermore, perhaps it would be good to distinghish suspended and
disabled add-ons:
Suspended is something temporary, due to global vars, and the add-ons
manager doesn't have the option to restart NVDA with suspended add-ons.
So it doesn't make sense to enable an add-on shown as suspended if it is
already enabled.
If you prefer, I could try to do a pull request on your branch based on
your work, since I don't have too much time, at least tomorrow. Or maybe
another people can continue testing this interesting feature.
Thanks.

El 01/03/2016 a las 10:46, 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).


Reply to this email directly or view it on GitHub
#3090 (comment).

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Mar 1, 2016

Contributor
Contributor

jcsteh commented Mar 1, 2016

@zahyur

This comment has been minimized.

Show comment
Hide comment
@zahyur

zahyur Mar 1, 2016

Hi,
I haven't looked at the code, but based on your comments:
I used the word "suspended" exactly to differentiate between add-on been disabled because of global flag and now worked disabling of individual add-ons. So if an add-on is disabled do to --disable-addons flag, it should say "suspended", and if disabled via state - "disabled".
So, if it's suspended, it should be possible to enable/disable it via state. It should be possible for pending operations to be reverted, i.e. "Pending disable" to be Enabled and "Pending Enabled" to be Disabled..

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.

zahyur commented Mar 1, 2016

Hi,
I haven't looked at the code, but based on your comments:
I used the word "suspended" exactly to differentiate between add-on been disabled because of global flag and now worked disabling of individual add-ons. So if an add-on is disabled do to --disable-addons flag, it should say "suspended", and if disabled via state - "disabled".
So, if it's suspended, it should be possible to enable/disable it via state. It should be possible for pending operations to be reverted, i.e. "Pending disable" to be Enabled and "Pending Enabled" to be Disabled..

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.

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Mar 2, 2016

Contributor

Hi, thanks for all your comments.
I think that, regarding to possibilities, Zahari is right.
In fact, command line could be used even to change any option in
configuration files, add any symbol or dictionary entry, remove or
install one or more add-ons, etc.
So I think it's useful to consider what can be needed to make a planing
to work
At this point, imo Jamie's suggestion is reasonable (I fix my previous
though about distinguishing suspended and disabled add-ons, if that
suggestion is followed).
About reverting pending states, we can discuss this in other issue,
though I think this isn't hight priority since, after restarting NVDA,
they can be reverted, and a confirmation message is presented before
proceed.
Thanks.

El 02/03/2016 a las 0:14, Zahari Yurukov escribió:

Hi,
I haven't looked at the code, but based on your comments:
I used the word "suspended" exactly to differentiate between add-on
been disabled because of global flag and now worked disabling of
individual add-ons. So if an add-on is disabled do to --disable-addons
flag, it should say "suspended", and if disabled via state - "disabled".
So, if it's suspended, it should be possible to enable/disable it via
state. It should be possible for pending operations to be reverted,
i.e. "Pending disable" to be Enabled and "Pending Enabled" to be
Disabled..

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.


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Mar 2, 2016

Hi, thanks for all your comments.
I think that, regarding to possibilities, Zahari is right.
In fact, command line could be used even to change any option in
configuration files, add any symbol or dictionary entry, remove or
install one or more add-ons, etc.
So I think it's useful to consider what can be needed to make a planing
to work
At this point, imo Jamie's suggestion is reasonable (I fix my previous
though about distinguishing suspended and disabled add-ons, if that
suggestion is followed).
About reverting pending states, we can discuss this in other issue,
though I think this isn't hight priority since, after restarting NVDA,
they can be reverted, and a confirmation message is presented before
proceed.
Thanks.

El 02/03/2016 a las 0:14, Zahari Yurukov escribió:

Hi,
I haven't looked at the code, but based on your comments:
I used the word "suspended" exactly to differentiate between add-on
been disabled because of global flag and now worked disabling of
individual add-ons. So if an add-on is disabled do to --disable-addons
flag, it should say "suspended", and if disabled via state - "disabled".
So, if it's suspended, it should be possible to enable/disable it via
state. It should be possible for pending operations to be reverted,
i.e. "Pending disable" to be Enabled and "Pending Enabled" to be
Disabled..

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.


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 2, 2016

Collaborator

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:

  • Security implications: NVDA can install anything from anywhere if given an opportunity (especially if forced from command line interface).
  • Internal identifier required: disabling add-ons from command line will require users to know internal identifier for a given add-on.
  • Capturing incorrect values: Perhaps users may spell an add-on identifier wrong or provide name of a nonexistent add-on. Although NVDA can handle this when add-ons are being disabled, it gets complex (punctuation must be checked, add-on identifiers must be checked, etc.).

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.

Collaborator

josephsl commented Mar 2, 2016

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:

  • Security implications: NVDA can install anything from anywhere if given an opportunity (especially if forced from command line interface).
  • Internal identifier required: disabling add-ons from command line will require users to know internal identifier for a given add-on.
  • Capturing incorrect values: Perhaps users may spell an add-on identifier wrong or provide name of a nonexistent add-on. Although NVDA can handle this when add-ons are being disabled, it gets complex (punctuation must be checked, add-on identifiers must be checked, etc.).

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.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Mar 2, 2016

Contributor
Contributor

jcsteh commented Mar 2, 2016

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Mar 2, 2016

Contributor

Hi, I agree with Jamie at this point: I think that it's better to allow
disabling add-ons while NVDA is restarted with disabled add-ons option.
I appreciated so much this command line option, and it can prevent problems.
If we start NVDA in normal mode and then try to disable selective
add-ons, it's possible that NVDA doesn't work at all.
I have needed visual assistance to delete the add-on subfolder since,
developing or testing add-ons, NVDA couldn't be started due to my tests
and modifications.
So I think it's necessary to implement Jamie's suggestion, since, in
this way, NVDA can be restarted in a sequre way and it's possible to
disable selectiva add-ons to find the problem.
Thanks.

El 02/03/2016 a las 7:49, josephsl 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:

  • Security implications: NVDA can install anything from anywhere if
    given an opportunity (especially if forced from command line
    interface).
  • Internal identifier required: disabling add-ons from command line
    will require users to know internal identifier for a given add-on.
  • Capturing incorrect values: Perhaps users may spell an add-on
    identifier wrong or provide name of a nonexistent add-on. Although
    NVDA can handle this when add-ons are being disabled, it gets
    complex (punctuation must be checked, add-on identifiers must be
    checked, etc.).

As for "suspended" versus "disabled": If I heard @jcsteh
https://github.com/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.


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Mar 2, 2016

Hi, I agree with Jamie at this point: I think that it's better to allow
disabling add-ons while NVDA is restarted with disabled add-ons option.
I appreciated so much this command line option, and it can prevent problems.
If we start NVDA in normal mode and then try to disable selective
add-ons, it's possible that NVDA doesn't work at all.
I have needed visual assistance to delete the add-on subfolder since,
developing or testing add-ons, NVDA couldn't be started due to my tests
and modifications.
So I think it's necessary to implement Jamie's suggestion, since, in
this way, NVDA can be restarted in a sequre way and it's possible to
disable selectiva add-ons to find the problem.
Thanks.

El 02/03/2016 a las 7:49, josephsl 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:

  • Security implications: NVDA can install anything from anywhere if
    given an opportunity (especially if forced from command line
    interface).
  • Internal identifier required: disabling add-ons from command line
    will require users to know internal identifier for a given add-on.
  • Capturing incorrect values: Perhaps users may spell an add-on
    identifier wrong or provide name of a nonexistent add-on. Although
    NVDA can handle this when add-ons are being disabled, it gets
    complex (punctuation must be checked, add-on identifiers must be
    checked, etc.).

As for "suspended" versus "disabled": If I heard @jcsteh
https://github.com/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.


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 2, 2016

Collaborator

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.

Collaborator

josephsl commented Mar 2, 2016

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.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 2, 2016

Collaborator

Hi,

One more thing: as for enable/disable button: would you like this to be kept or dropped? I'd say let's keep it for consistency and there might be times when users wish to disable certain add-ons while NVDA is in normal mode. Thanks.

Collaborator

josephsl commented Mar 2, 2016

Hi,

One more thing: as for enable/disable button: would you like this to be kept or dropped? I'd say let's keep it for consistency and there might be times when users wish to disable certain add-ons while NVDA is in normal mode. Thanks.

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Mar 2, 2016

Contributor

I will let it keept.
Thanks.

El 02/03/2016 a las 8:29, josephsl escribió:

Hi,

One more thing: as for enable/disable button: would you like this to
be kept or dropped? I'd say let's keep it for consistency and there
might be times when users wish to disable certain add-ons while NVDA
is in normal mode. Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Mar 2, 2016

I will let it keept.
Thanks.

El 02/03/2016 a las 8:29, josephsl escribió:

Hi,

One more thing: as for enable/disable button: would you like this to
be kept or dropped? I'd say let's keep it for consistency and there
might be times when users wish to disable certain add-ons while NVDA
is in normal mode. Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment).

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Mar 2, 2016

Contributor
Contributor

jcsteh commented Mar 2, 2016

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 2, 2016

Collaborator

Hi,

Compiling with latest master applied was a success...

It turns out there's nothing to do - Jamie's suggestion has been implemented without me knowing it (disable button does show up when you open add-ons manager while all add-ons are disabled). Thanks.

Collaborator

josephsl commented Mar 2, 2016

Hi,

Compiling with latest master applied was a success...

It turns out there's nothing to do - Jamie's suggestion has been implemented without me knowing it (disable button does show up when you open add-ons manager while all add-ons are disabled). Thanks.

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Mar 2, 2016

Contributor

Hi, when all add-ons are disabled, and also an add-on was already
disabled, the disable add-on button is shown too. I think it shouldn't.
Also, when an add-on is enabled, the focus doesn't return to the add-on
list.
Thanks.

El 02/03/2016 a las 9:05, josephsl escribió:

Hi,

Compiling with latest master applied was a success...

It turns out there's nothing to do - Jamie's suggestion has been
implemented without me knowing it (disable button does show up when
you open add-ons manager while all add-ons are disabled). Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment).

Contributor

nvdaes commented Mar 2, 2016

Hi, when all add-ons are disabled, and also an add-on was already
disabled, the disable add-on button is shown too. I think it shouldn't.
Also, when an add-on is enabled, the focus doesn't return to the add-on
list.
Thanks.

El 02/03/2016 a las 9:05, josephsl escribió:

Hi,

Compiling with latest master applied was a success...

It turns out there's nothing to do - Jamie's suggestion has been
implemented without me knowing it (disable button does show up when
you open add-ons manager while all add-ons are disabled). Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment).

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Mar 2, 2016

Collaborator

Hi, latest commit to i3090 provided a fix for it (the commit before merging master/VC2015).

From: nvdaes [mailto:notifications@github.com]
Sent: Wednesday, March 2, 2016 12:29 AM
To: nvaccess/nvda nvda@noreply.github.com
Cc: josephsl joseph.lee22590@gmail.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

Hi, when all add-ons are disabled, and also an add-on was already
disabled, the disable add-on button is shown too. I think it shouldn't.
Also, when an add-on is enabled, the focus doesn't return to the add-on
list.
Thanks.

El 02/03/2016 a las 9:05, josephsl escribió:

Hi,

Compiling with latest master applied was a success...

It turns out there's nothing to do - Jamie's suggestion has been
implemented without me knowing it (disable button does show up when
you open add-ons manager while all add-ons are disabled). Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment).


Reply to this email directly or view it on GitHub #3090 (comment) .

Collaborator

josephsl commented Mar 2, 2016

Hi, latest commit to i3090 provided a fix for it (the commit before merging master/VC2015).

From: nvdaes [mailto:notifications@github.com]
Sent: Wednesday, March 2, 2016 12:29 AM
To: nvaccess/nvda nvda@noreply.github.com
Cc: josephsl joseph.lee22590@gmail.com
Subject: Re: [nvda] add enable/disable function in addons manager. (#3090)

Hi, when all add-ons are disabled, and also an add-on was already
disabled, the disable add-on button is shown too. I think it shouldn't.
Also, when an add-on is enabled, the focus doesn't return to the add-on
list.
Thanks.

El 02/03/2016 a las 9:05, josephsl escribió:

Hi,

Compiling with latest master applied was a success...

It turns out there's nothing to do - Jamie's suggestion has been
implemented without me knowing it (disable button does show up when
you open add-ons manager while all add-ons are disabled). Thanks.


Reply to this email directly or view it on GitHub
#3090 (comment).


Reply to this email directly or view it on GitHub #3090 (comment) .

josephsl added a commit to josephsl/nvda that referenced this issue Jun 16, 2016

josephsl added a commit to josephsl/nvda that referenced this issue Jun 16, 2016

Add-ons manager: Changed UI string for diable add-on dialog and resta…
…rt screen. re #3090

Good catch by Derek Riemer: always strive for consistency in user interface strings.

josephsl added a commit to josephsl/nvda that referenced this issue Jun 30, 2016

Add-ons manager: do not give a warning message when enabling or disab…
…ling add-ons, simplify restart prompt. re #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.

josephsl added a commit to josephsl/nvda that referenced this issue Jun 30, 2016

Add-ons manager: Just say 'running' when re-enabling an add-on withou…
…t a restart. re #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.

josephsl added a commit to josephsl/nvda that referenced this issue Jul 5, 2016

Add-ons manager: just check pendingDisable once. re #3090.
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.

josephsl added a commit to josephsl/nvda that referenced this issue Jul 5, 2016

Add-ons manager: refactored how needs restart flag is set by checking…
… for cardinality of sets and comparing sets. re #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.

josephsl added a commit to josephsl/nvda that referenced this issue Jul 5, 2016

Add-ons manager and add-ons subsystem: Persistence logic in addonHand…
…ler.terminate, rewrote needsRestart. re #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).

josephsl added a commit to josephsl/nvda that referenced this issue Jul 6, 2016

Add-ons manager: Record names of add-ons that are disabled in hopes t…
…hat we have a persistent backup in case of disasters such as crashes. re #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.

josephsl added a commit to josephsl/nvda that referenced this issue Jul 6, 2016

Add-ons Manager/optimization: due to use of states, no need to use ru…
…ntime states denoting future enable/disable. re #3090

josephsl added a commit to josephsl/nvda that referenced this issue Jul 7, 2016

josephsl added a commit to josephsl/nvda that referenced this issue Jul 8, 2016

josephsl added a commit to josephsl/nvda that referenced this issue Jul 8, 2016

Revert "Add-ons manager: show enable if and only if add-on is suspend…
…ed and is pending enable. re #3090"

This reverts commit f4e3852.

@jcsteh jcsteh added this to the 2016.3 milestone Aug 5, 2016

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 5, 2016

Contributor

Merged in d9b3ac1.

Contributor

jcsteh commented Aug 5, 2016

Merged in d9b3ac1.

@jcsteh jcsteh closed this Aug 5, 2016

@zahyur

This comment has been minimized.

Show comment
Hide comment
@zahyur

zahyur Aug 23, 2016

Hi,
the following line is from the NVDA's user guide:
You can disable an add-on if the add-on status indicates it is running or enabled, or enable it if the add-on is suspended or disabled.

Shouldn't it be:
You can disable an add-on if the add-on status indicates it is running, suspended or enabled, or enable it if the add-on is disabled.

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.

zahyur commented Aug 23, 2016

Hi,
the following line is from the NVDA's user guide:
You can disable an add-on if the add-on status indicates it is running or enabled, or enable it if the add-on is suspended or disabled.

Shouldn't it be:
You can disable an add-on if the add-on status indicates it is running, suspended or enabled, or enable it if the add-on is disabled.

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.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 23, 2016

Contributor

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.

Contributor

jcsteh commented Aug 23, 2016

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.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 23, 2016

Contributor

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.

Contributor

jcsteh commented Aug 23, 2016

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.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Aug 23, 2016

Collaborator

Hi, will wait for a follow-up issue before posting a possible PR. Thanks.

From: James Teh [mailto:notifications@github.com]
Sent: Tuesday, August 23, 2016 4:28 PM
To: nvaccess/nvda nvda@noreply.github.com
Cc: Joseph Lee joseph.lee22590@gmail.com; Assign assign@noreply.github.com
Subject: Re: [nvaccess/nvda] add enable/disable function in addons manager. (#3090)

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.


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub #3090 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AHgLkOyH_Dt7hJhfB9FwkLI8NfHZYp1Hks5qi4IUgaJpZM4Hkv8I .

Collaborator

josephsl commented Aug 23, 2016

Hi, will wait for a follow-up issue before posting a possible PR. Thanks.

From: James Teh [mailto:notifications@github.com]
Sent: Tuesday, August 23, 2016 4:28 PM
To: nvaccess/nvda nvda@noreply.github.com
Cc: Joseph Lee joseph.lee22590@gmail.com; Assign assign@noreply.github.com
Subject: Re: [nvaccess/nvda] add enable/disable function in addons manager. (#3090)

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.


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub #3090 (comment) , or mute the thread https://github.com/notifications/unsubscribe-auth/AHgLkOyH_Dt7hJhfB9FwkLI8NfHZYp1Hks5qi4IUgaJpZM4Hkv8I .

@zahyur

This comment has been minimized.

Show comment
Hide comment
@zahyur

zahyur Aug 24, 2016

Hi,
I haven't tried this, but in Firefox, it probably isn't possible to disable add-ons globally, then to disable individual add-ons in the same session with add-ons disabled globally. It may even be possible to enable them at this point.

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,
Zahari

zahyur commented Aug 24, 2016

Hi,
I haven't tried this, but in Firefox, it probably isn't possible to disable add-ons globally, then to disable individual add-ons in the same session with add-ons disabled globally. It may even be possible to enable them at this point.

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,
Zahari

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 24, 2016

Contributor
Contributor

jcsteh commented Aug 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment