Closed Bug 702920 Opened 13 years ago Closed 12 years ago

If compatibility changes for a user-disabled addon, the UI doesn't get notified and doesn't update

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: Unfocused, Assigned: Unfocused)

References

Details

Attachments

(1 file)

If compatibility of a user-disabled addon changes (eg, compatible->incompatible) while the UI is open, the UI never gets notified of the change and thus doesn't update to show the new compatibility status.

This can occur when a compatibility-related pref changes - eg, extensions.strictCompatibility.

Note that it doesn't occur when there's a compatibility update, as the UI gets notified via onCompatibilityUpdateAvailable.
I wonder if we could just abuse onCompatibilityUpdateAvailable and send that when the compatibility changes. Not sure what other side-effects that would have though.
Blocks: 693901
How about sending "onPropertyChanged" for isCompatible or appDisabled?
Yea, that would work.
Attached patch Patch v1Splinter Review
And as usual, this is mostly tests.
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
Attachment #582714 - Flags: review?(dtownsend)
Comment on attachment 582714 [details] [diff] [review]
Patch v1

Review of attachment 582714 [details] [diff] [review]:
-----------------------------------------------------------------

The tests here verify that the UI updates when the onPropertyChanged event is sent, it doesn't verify that that event is actually sent for real add-ons. I think this needs an additional test for that. Install a real add-on and toggle strictCompatibility or something. xpcshell might be easiest for that.

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +3761,5 @@
> +    if (appDisabledChanged) {
> +      AddonManagerPrivate.callAddonListeners("onPropertyChanged",
> +                                            aAddon,
> +                                            ["appDisabled"]);
> +    }

Might make sense to include notifications for the other properties too. Up to you.
Attachment #582714 - Flags: review?(dtownsend) → review-
(In reply to Dave Townsend (:Mossop) from comment #7)
> The tests here verify that the UI updates when the onPropertyChanged event
> is sent, it doesn't verify that that event is actually sent for real
> add-ons. I think this needs an additional test for that. Install a real
> add-on and toggle strictCompatibility or something. xpcshell might be
> easiest for that.

Already done, see test_onPropertyChanged_appDisabled.js in that patch.


> Might make sense to include notifications for the other properties too. Up
> to you.

Looked into doing this:
- There's no way to tell if isCompatible changed, since it does so before updateAddonDisabledState is called, and depends on so many things
- The only time softDisabled can change is in cases where onEnabling/onDisabling are also fired
- Firing onPropertyChanged for userDisabled requires far more work than it's worth

So I'll skip that until there's an actual need for any of those.
Attachment #582714 - Flags: review- → review+
Attachment #582714 - Flags: review+ → review?(dtownsend)
Attachment #582714 - Flags: review?(dtownsend) → review+
https://hg.mozilla.org/integration/fx-team/rev/d0b03b763e5b
Flags: in-testsuite+
Flags: in-litmus-
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla12
Backed out because of xpcshell oranges:

https://hg.mozilla.org/integration/fx-team/rev/3698bfa55a72
Whiteboard: [fixed-in-fx-team]
Oh, that was stupid... this is dependent on bug 715787, which hasn't landed yet.
Whiteboard: [needs bug 715787]
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe532188a808
Whiteboard: [needs bug 715787]
Target Milestone: mozilla12 → mozilla13
https://hg.mozilla.org/mozilla-central/rev/fe532188a808
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: