519671 - Impossible to customize the thread summary head panel buttons appearance

Status: RESOLVED DUPLICATE of bug 1772911

(Thunderbird :: Message Reader UI, defect)

Description

[Valery Kondakoff]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.4pre) Gecko/20090929 Shredder/3.0pre

It is impossible to customize the thread summary head panel buttons appearance (Icons/Text, Icons, Text) by right-clicking on them. Should they follow the settings of Message view header pane?

Reproducible: Always

Comment 1

[Bryan Clark (DevTools PM) [:clarkbw]]

Might make sense to do that for people who are trying to save some space.  Space in this areas is much less of a concern because things wrap well so I wouldn't block the release on getting this done.  Certainly would accept patches

Comment 2

[Jim Porter (:squib)]

Attachment: Really (really!) messy patch to fix this

I realized that this is possible now with the HTML5 context menus, and some other tricks, like overriding document.persist(). Here's a patch to make this work. We should probably add more buttons to the palette, but this is good enough for a proof of concept for now.

Comment 3

[Jim Porter (:squib)]

Attachment: Cleaned up patch

Here's a cleaner version of the patch.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 617264 [details] [diff] [review]
Cleaned up patch

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

Jim:

Cool stuff.

As we tend towards more XHTML and less XUL, I can see this sort of thing becoming more and more common - so I can see the helper you've created being pretty useful elsewhere as well.

A few notes:

1)  It looks like we're doing synchronous IO for the JSON file holding the toolbar persistence.  We might want to switch to async, to protect ourselves from UI stickiness.
2)  Or should we be sticking this sort of thing in prefs as a complex value instead?  Is there value in doing it with the .json file instead?
3)  I'm a little wary of overwriting the persist function for the entire document for a single toolbar... not a showstopper, but something to keep in mind for the future - we might want a more general framework for persisting stuff in XHTML documents.
4)  Why do we need to recreate those strings?

-Mike

::: mail/base/content/multimessageview.xhtml
@@ +112,4 @@
>                     xmlns:html="http://www.w3.org/1999/xhtml"
>                     xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
> +        <toolbarpalette id="bar"
> +          ><toolbarbutton id="hdrArchiveButton"

Pretty funky open/close tag stylings in here.  :D

Comment 5

[Jim Porter (:squib)]

(In reply to Mike Conley (:mconley) from comment #4)
> 1)  It looks like we're doing synchronous IO for the JSON file holding the
> toolbar persistence.  We might want to switch to async, to protect ourselves
> from UI stickiness.

That's probably a smart move.

> 2)  Or should we be sticking this sort of thing in prefs as a complex value
> instead?  Is there value in doing it with the .json file instead?

We could, but I was going for consistency with localStore.rdf, which is where stuff is normally persisted. It also makes it easier to debug by having a proper file to hand-edit.

> 3)  I'm a little wary of overwriting the persist function for the entire
> document for a single toolbar... not a showstopper, but something to keep in
> mind for the future - we might want a more general framework for persisting
> stuff in XHTML documents.

document.persist only exists for XUL documents, so we're really just adding a new function to the HTML document that works like XUL. The Customize Toolbars dialog expects this function to be present; we could change that, but I'm not sure how generally useful that would be, since it's only an issue when you have XUL-in-HTML.

> 4)  Why do we need to recreate those strings?

I renamed the various string keys to be consistent with the single-message header. I'm not using the single-message header's strings because the tooltips are different, and some locales might want different strings (singular vs plural).

> ::: mail/base/content/multimessageview.xhtml
> @@ +112,4 @@
> >                     xmlns:html="http://www.w3.org/1999/xhtml"
> >                     xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
> > +        <toolbarpalette id="bar"
> > +          ><toolbarbutton id="hdrArchiveButton"
> 
> Pretty funky open/close tag stylings in here.  :D

Yeah, XUL-in-HTML is really wonderful. If you don't do that, the Customize Toolbar dialog dies because it sees a bunch of extra text nodes.

Comment 6

[Jim Porter (:squib)]

Attachment: Updated patch

Here's an updated patch with a better API, some fixed bugs, async IO, and written on top of my patches in bug 942638. Still needs tests and such, though.

Comment 7

[Jim Porter (:squib)]

I'm wondering if it will be easier to do this when localstore.rdf is deprecated...?