mozilla / nightlytt

Nightly Tester Tools
https://wiki.mozilla.org/Auto-tools/Automation_Development/Projects/Addons/NightlyTesterTools
Other
61 stars 37 forks source link

No more FUEL, no more STEEL #206

Closed xabolcs closed 8 years ago

xabolcs commented 8 years ago

This is a fix for Issue #205.

Cc @whimboo

xabolcs commented 8 years ago

Branch updated with commit ec6ec039741cbf402b5bedaff7b284f79b08700a.

whimboo commented 8 years ago

Hm, I tried to check those changes with the toggleCompatibility() feature, but when I flip the checkmark in the menu, I do not see the popupnotification in latest Nightly. Is that another issue we would have to fix beside that one?

xabolcs commented 8 years ago

You need an enabled incompatible addon which will be enabled / disabled when you toggle compatibility checks.

If there is no need to restart then no notification will pop up.

whimboo commented 8 years ago

Hm, that isn't clear to me anymore while it makes sense for sure. Where are we doing this check? I had a look at toggleCompatibility but I wasn't able to find it there.

Sorry for pestering with those questions but it's a long time we did some updates for NTT. :S

xabolcs commented 8 years ago

No problem! :)

It's for the restartful addons. Here is the count() function that counts the addons which requires a restart. The result of the above callback is used to decide wether to do a restart or not.

It makes sense when the user's incompatible addons are all restartless. They get started instantly when the force compatibility toggles ... and no need for restart.

That result is parsed in toggleCompatibility(), to do the real restart or not to do.

whimboo commented 8 years ago

I see, so we do it via the PREF_FORCE_COMPAT observer notification. I think that will help me now to get this tested.

xabolcs commented 8 years ago

Sure. For example you need a simple count like in the fallback ... or simple leave out the parsedData.restart part. :)

whimboo commented 8 years ago

Tested with DownThemAll and it works perfect. I also cannot see anything else which would need a fix. Lets get this merged.

xabolcs commented 8 years ago

Commit f08b18f doesn't have the commit message that you really wanted. :-1: But the one-click-squash-and-merge stuff is really cool! :+1:

whimboo commented 8 years ago

Yeah, I somehow messed it up. sorry. I will pay more attention the next time. I also used it only once so far.

xabolcs commented 8 years ago

No problem, just wanted to note! :)