customizableui-areatype is not reset during initial population

RESOLVED FIXED in Firefox 28

Status

()

Firefox
Toolbars and Customization
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mak, Assigned: mconley)

Tracking

unspecified
Firefox 28
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M8][Australis:P1])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
bug 899576 added default customizableui-areatype attribute to a bunch of items, though if those items are initially in the palette, they end up with the wrong attribute and the wrong rules apply to them. the attribute should be reset when initially populating the palette.
This should be a piece of cake - just waiting for build to finish, and then I'll post a patch.
Whiteboard: [Australis:M?][Australis:P1]
Created attachment 785154 [details] [diff] [review]
Patch v1

Should do the job.
Attachment #785154 - Flags: review?(gijskruitbosch+bugs)

Comment 3

5 years ago
Comment on attachment 785154 [details] [diff] [review]
Patch v1

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

(In reply to Mike Conley (:mconley) from comment #2)
> Created attachment 785154 [details] [diff] [review]
> Patch v1
> 
> Should do the job.

Can we not do this the way Marco suggested, when we enter customization mode? Otherwise, r=me, although the paranoid part of me wonders whether it matters whether you do this before or after we append to the palette (and I suspect after is better, as the palette is detached from the DOM).
Attachment #785154 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 785154 [details] [diff] [review]
Patch v1

As I kinda fed the bug details to Marco over IRC, I don't think he meant to remove the attribute when populating the palette on entering customization mode - he (I) meant when populating the palette with stuff that is being removed from toolbars.

But I kind of like this idea of stripping the attributes when populating the visible palette. I'll whip up a patch.
Attachment #785154 - Attachment is obsolete: true
Erg, but then that means exposing removeLocationAttributes through CustomizableUI to CustomizeMode. :/ I like that less now...
Created attachment 785175 [details] [diff] [review]
Patch v1.1

This seems simplest, and removes the attribute after we remove the node.
Attachment #785175 - Flags: review?(gijskruitbosch+bugs)

Comment 7

5 years ago
Comment on attachment 785175 [details] [diff] [review]
Patch v1.1

We really need to sort out the CustomizeMode/CustomizableUI wrapping/attribute/thingymajig, but that's a separate bug. This WFM, r=me.
Attachment #785175 - Flags: review?(gijskruitbosch+bugs) → review+
Yeppers.

https://hg.mozilla.org/projects/ux/rev/c19d88756321

Thanks.
Whiteboard: [Australis:M?][Australis:P1] → [Australis:M?][Australis:P1][fixed-in-ux]
Whiteboard: [Australis:M?][Australis:P1][fixed-in-ux] → [Australis:M8][Australis:P1][fixed-in-ux]

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/c19d88756321
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M8][Australis:P1][fixed-in-ux] → [Australis:M8][Australis:P1]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.