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

When disabling and then immediately re-enabling an add-on or vice versa, the add-on status now correctly reverts to what it was previously. #6299

Merged
merged 1 commit into from Sep 16, 2016

Conversation

Projects
None yet
4 participants
@jcsteh
Contributor

jcsteh commented Aug 24, 2016

When an add-on is in the pending disable set and the user enables it, the user is just undoing the disable; i.e. there is nothing to do. Therefore, just remove it from the pending disable set; don't also add it to the pending enable set. Make a similar change for undoing an enable.

When disabling and then immediately re-enabling an add-on or vice ver…
…sa, the add-on status now correctly reverts to what it was previously.

When an add-on is in the pending disable set and the user enables it, the user is just undoing the disable; i.e. there is nothing to do. Therefore, just remove it from the pending disable set; don't also add it to the pending enable set. Make a similar change for undoing an enable.
@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 24, 2016

Contributor

STR:

  1. Ensure there is at least one add-on installed.
  2. Open the Add-ons Manager.
  3. Select an add-on.
  4. Press Disable.
  5. Immediately press Enable.
    • Expected: The status should revert to "running".
    • Result before the fix: The status is "enable".
Contributor

jcsteh commented Aug 24, 2016

STR:

  1. Ensure there is at least one add-on installed.
  2. Open the Add-ons Manager.
  3. Select an add-on.
  4. Press Disable.
  5. Immediately press Enable.
    • Expected: The status should revert to "running".
    • Result before the fix: The status is "enable".
@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 24, 2016

Contributor

@feerrenrut, can you review please? Thanks!

CC @josephsl.

Contributor

jcsteh commented Aug 24, 2016

@feerrenrut, can you review please? Thanks!

CC @josephsl.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Aug 24, 2016

Collaborator

Hi, in regards to user guide, should the possibly misleading sentence be removed? Thanks.

From: James Teh [mailto:notifications@github.com]
Sent: Tuesday, August 23, 2016 8:51 PM
To: nvaccess/nvda nvda@noreply.github.com
Cc: Joseph Lee joseph.lee22590@gmail.com; Mention mention@noreply.github.com
Subject: Re: [nvaccess/nvda] When disabling and then immediately re-enabling an add-on or vice versa, the add-on status now correctly reverts to what it was previously. (#6299)

@feerrenrut https://github.com/feerrenrut , can you review please? Thanks!

CC @josephsl https://github.com/josephsl .


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

Collaborator

josephsl commented Aug 24, 2016

Hi, in regards to user guide, should the possibly misleading sentence be removed? Thanks.

From: James Teh [mailto:notifications@github.com]
Sent: Tuesday, August 23, 2016 8:51 PM
To: nvaccess/nvda nvda@noreply.github.com
Cc: Joseph Lee joseph.lee22590@gmail.com; Mention mention@noreply.github.com
Subject: Re: [nvaccess/nvda] When disabling and then immediately re-enabling an add-on or vice versa, the add-on status now correctly reverts to what it was previously. (#6299)

@feerrenrut https://github.com/feerrenrut , can you review please? Thanks!

CC @josephsl https://github.com/josephsl .


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

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Aug 24, 2016

Contributor
Contributor

jcsteh commented Aug 24, 2016

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Aug 24, 2016

Collaborator

Hi, confirmed the STR. Thanks.

From: James Teh [mailto:notifications@github.com]
Sent: Tuesday, August 23, 2016 9:12 PM
To: nvaccess/nvda nvda@noreply.github.com
Cc: Joseph Lee joseph.lee22590@gmail.com; Mention mention@noreply.github.com
Subject: Re: [nvaccess/nvda] When disabling and then immediately re-enabling an add-on or vice versa, the add-on status now correctly reverts to what it was previously. (#6299)

That's a separate issue I think. See the steps to reproduce earlier in
this issue for details on what this PR is fixing.


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

Collaborator

josephsl commented Aug 24, 2016

Hi, confirmed the STR. Thanks.

From: James Teh [mailto:notifications@github.com]
Sent: Tuesday, August 23, 2016 9:12 PM
To: nvaccess/nvda nvda@noreply.github.com
Cc: Joseph Lee joseph.lee22590@gmail.com; Mention mention@noreply.github.com
Subject: Re: [nvaccess/nvda] When disabling and then immediately re-enabling an add-on or vice versa, the add-on status now correctly reverts to what it was previously. (#6299)

That's a separate issue I think. See the steps to reproduce earlier in
this issue for details on what this PR is fixing.


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

@feerrenrut

This comment has been minimized.

Show comment
Hide comment
@feerrenrut

feerrenrut Aug 29, 2016

Contributor

Yep, change looks good to me.

Contributor

feerrenrut commented Aug 29, 2016

Yep, change looks good to me.

jcsteh added a commit that referenced this pull request Aug 29, 2016

@nvaccessAuto nvaccessAuto assigned jcsteh and unassigned feerrenrut Aug 29, 2016

@jcsteh jcsteh merged commit 05d74c7 into master Sep 16, 2016

@nvaccessAuto nvaccessAuto added this to the 2016.4 milestone Sep 16, 2016

@jcsteh jcsteh deleted the undoAddonDisable branch Sep 16, 2016

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