mozilla / personas-plus

Personas Plus extension for Firefox
https://addons.mozilla.org/addon/personas-plus/
Mozilla Public License 2.0
7 stars 11 forks source link

Theme Info Bar Undo Fix #83

Closed derinb closed 7 years ago

derinb commented 7 years ago

Theme info bar can be dismissed by clicking the Undo button after uninstalling PP by this commit. Fixes #82

wagnerand commented 7 years ago

Hm, I don't understand how that patch resolves the issue. Can you explain, please?

derinb commented 7 years ago

When uninstalled or disabled, all scripts unloaded. PersonaService is not available any more. If clicked, error is thrown which prevents the execution to successfully complete attached Undo callback. That is why Undo does not dismiss the notification bar. It halts at

PersonaService.revertToPreviousPersona();

line.

By try and catch block, we can detect the uninstall and disable case. If there is no PersonaService available, nothing is executed and notification bar is simply dismissed.

Alternatively, an add-on listener should be attached for listening uninstall and disable which would close all notification bars automatically. But it is not necessary to add extra memory for such a rare bug. Try and catch is enough for this minor bug.

wagnerand commented 7 years ago

Alternatively, an add-on listener should be attached for listening uninstall and disable which would close all notification bars automatically. But it is not necessary to add extra memory for such a rare bug.

This is the way we should be going. The add-on is responsible for adding the notification, so it should be for cleaning it up. Also, it's quite surprising if you click on the 'Undo' button and nothing happens.

It shouldn't be a big deal to check the notification bar in the uninstall handler. I don't think we would need extra memory for that?

derinb commented 7 years ago

Updated. Removing all notifications bar at shutdown automatically.

derinb commented 7 years ago

Updated. Thanks

wagnerand commented 7 years ago

Great, thank you!