Closed Bug 1546340 Opened 6 years ago Closed 5 years ago

[de-xbl] remove the (forked) toolbarpaletteitem and toolbarpaletteitem-palette bindings

Categories

(Thunderbird :: Toolbars and Tabs, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 69.0

People

(Reporter: mkmelin, Assigned: aleca)

References

Details

Attachments

(1 file, 1 obsolete file)

Remove the toolbarpaletteitem binding: https://searchfox.org/comm-central/rev/024094c2ebc05723d2b0da86086539d5b97044cb/common/bindings/toolbar.xml#541

This was done in bug 1507875 for Firefox https://hg.mozilla.org/mozilla-central/rev/8f7476054232, and we should do the same. The commit comment explains the story.

Looks like toolbarpaletteitem-palette would go at the same time
https://searchfox.org/comm-central/rev/024094c2ebc05723d2b0da86086539d5b97044cb/mail/base/content/bindings.css#32 (some adjustments may be needed)

Summary: [de-xbl] remove the (forked) toolbarpaletteitem binding → [de-xbl] remove the (forked) toolbarpaletteitem and toolbarpaletteitem-palette bindings
Attached patch dexbl-toolbarpaletteitem.patch (obsolete) (deleted) β€” β€” Splinter Review

I think I got all the CSS updates and implemented the same approach done in FF.
I can't seem to figure out what can be removed from the unwrapToolbarItems() method as the outcome seems to be always the same even if I completely remove that method.
Do you know what's the purpose of that method and how is supposedly changing the markup?

Attachment #9069550 - Flags: feedback?(mkmelin+mozilla)
Status: NEW → ASSIGNED
Comment on attachment 9069550 [details] [diff] [review]
dexbl-toolbarpaletteitem.patch

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

Looks ok. Maybe unwrapToolbarItem doesn't need to be adjusted. I don't know if it's really needed...
Attachment #9069550 - Flags: feedback?(mkmelin+mozilla) → feedback+
Attached patch dexbl-toolbarpaletteitem.patch (deleted) β€” β€” Splinter Review

Nevermind, the unwrapToolbarItems is necessary to make the toolbar buttons usable once added. Without it, the drag and drop overlay doesn't get removed.

This should be ready for a review.

Attachment #9069550 - Attachment is obsolete: true
Attachment #9073056 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9073056 [details] [diff] [review]
dexbl-toolbarpaletteitem.patch

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

Looks good to me, r=mkmelin
Attachment #9073056 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/451a275be8a6
[de-xbl] remove toolbarpaletteitem and toolbarpaletteitem-palette bindings. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 69.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: