Closed Bug 611559 Opened 13 years ago Closed 13 years ago

Backgrounds never have PERM_CAN_DISABLE, even when they can be disabled

Categories

(Toolkit :: Add-ons Manager, defect)

Product:
Toolkit
Component:
Add-ons Manager
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla2.0
Tracking Flags:
Tracking Status
blocking2.0 --- -

People

(Reporter: Unfocused, Assigned: Unfocused)

References

Details

(Whiteboard: [addons-testday])

Attachments

(1 file)

Patch v1
16.51 KB, patch
mossop
: review+
mossop
: approval2.0+
Details | Diff | Splinter Review 
Backgrounds (lightweight themes) can be disabled like any addon type now, but they never advertise that via PERM_CAN_DISABLE. So the UI never thinks a background can be disabled.
blocking2.0: --- → ?
Blocks: 597607
I'm not sure this blocks. Maybe I'm not understanding it though.

Isn't not using a theme effectively disabling it? What's the use-case in the UI for a user disabling a theme, as distinct from just selecting a different theme?
Its for the "I don't want a background any more" use case. Currently you need to enable the theme you already have enabled (but shows as disabled because there's a background enabled). Its simpler to just disable the background that is currently enabled.

The UI already has all the code and strings for this - and the API already supports disabling backgrounds (see bug 553494 comment 17 - this bug something that was overlooked there), it just doesn't advertize that to the UI.
Not going to block as the UI works ok without it. Would probably take a safe patch for it before b9 though.
blocking2.0: ? → -
Attached patch Patch v1 — — Splinter Review 
A bit later than I wanted, but its tested well :)
Assignee: nobody → bmcbride
Status: NEW → ASSIGNED
Attachment #513432 - Flags: review?(dtownsend)
Comment on attachment 513432 [details] [diff] [review]
Patch v1

Straightforward enough I guess, bounce it at the first sign of regression though.
Attachment #513432 - Flags: review?(dtownsend)
Attachment #513432 - Flags: review+
Attachment #513432 - Flags: approval2.0+
Keywords: checkin-needed
Whiteboard: [has patch][needs landing]
http://hg.mozilla.org/mozilla-central/rev/7394613d0750
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][needs landing] → [has patch]
Target Milestone: --- → mozilla2.0
Flags: in-litmus-
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0
Status: RESOLVED → VERIFIED
Whiteboard: [has patch] → [addons-testday]
You need to log in before you can comment on or make changes to this bug.