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)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: Unfocused, Assigned: Unfocused)
References
Details
Attachments
(1 file)
12.27 KB,
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•13 years ago
|
||
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.
Comment 2•13 years ago
|
||
How about sending "onPropertyChanged" for isCompatible or appDisabled?
Assignee | ||
Comment 3•13 years ago
|
||
Yea, that would work.
Assignee | ||
Comment 6•12 years ago
|
||
And as usual, this is mostly tests.
Comment 7•12 years ago
|
||
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-
Assignee | ||
Comment 8•12 years ago
|
||
(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.
Assignee | ||
Updated•12 years ago
|
Attachment #582714 -
Flags: review- → review+
Assignee | ||
Updated•12 years ago
|
Attachment #582714 -
Flags: review+ → review?(dtownsend)
Updated•12 years ago
|
Attachment #582714 -
Flags: review?(dtownsend) → review+
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d0b03b763e5b
Flags: in-testsuite+
Flags: in-litmus-
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla12
Comment 10•12 years ago
|
||
Backed out because of xpcshell oranges: https://hg.mozilla.org/integration/fx-team/rev/3698bfa55a72
Whiteboard: [fixed-in-fx-team]
Assignee | ||
Comment 11•12 years ago
|
||
Oh, that was stupid... this is dependent on bug 715787, which hasn't landed yet.
Assignee | ||
Updated•12 years ago
|
Whiteboard: [needs bug 715787]
Assignee | ||
Comment 12•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe532188a808
Whiteboard: [needs bug 715787]
Target Milestone: mozilla12 → mozilla13
Comment 13•12 years ago
|
||
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.
Description
•