At the moment the toolbars in the Composer and MailNews Composition windows are not customizable - that should be fixed.

I did look at putting on an ignore whitespace change patch but less than 1K smaller.

Comment on attachment 485506 [details] [diff] [review] Make toolbars customizable patch v0.1 I have not looked at the code (except when I tracked the console warning), so this is more of a ui-behaviour feedback. Overall, it works fine - nice job :-) But there are a few things that we need to settle: 1: The format toolbar buttons have no label text (and we don't want them to have label text). The icons also only comes in one size (anything else would of course be strange) so the choices for "Settings for this toolbar" doesn't make sense here. 2: When switching between modes (normal, html tags, html source, preview), your choice of showing/hiding the format toolbar doesn't persist (try for example to hide the format toolbar in "normal" mode, then switch to "html tags" and back). I'm also not clear how we should handle the format toolbar in the different modes - but we should probably treat all modes as they have the same toolbar ("persisting through modes")? Need then to consider the fact that the source mode view then would have a disabled format toolbar... :-/ 3: Showing/hiding a toolbar by selecting/deselecting in the context menu doesn't update the menuitem in the View menu. Doing it from the View Menu updates the context menuitems, though 4: When you have the customize sheet in front of you, it's possible to type in the editable area - but you knew that already since we talked about it :-) 5: - gEditorToolbarPrefListener = new nsPrefListener(kEditorToolbarPrefs); EditorShutdown() does gEditorToolbarPrefListener.shutdown(), so I get a warning in the error console when closing the editor window: ---------------------------------------------- Error: gEditorToolbarPrefListener is undefined Source File: chrome://editor/content/editor.js Line: 673 ---------------------------------------------- Minusing since I think we need to settle the behaviour in 1-4 first.

Comment on attachment 485506 [details] [diff] [review] Make toolbars customizable patch v0.1 Sorry for the delay, first had to get a clean tree... (In reply to comment #0) > At the moment the toolbars in the Composer and MailNews Composition windows are > not customizable AFAICS MailNews compose windows are already partly customizable, "only" the ability to customize the Formatting Toolbar was added. Care to update the bug summary (which will probably also help the check-in comment)? Just asking, maybe I'm wrong. ;-) It seems Stefan already did a good job at checking the behavior (much better than what I would have come up with, in fact). Since I'm generally happy with it I give a f+ if you address the View menu issue, but if in doubt, take Stefan's feedback (check-in comment etc.). One more To Do for you (just FTR, I'm sure you'd have taken care of that eventually): Help needs to be adapted, either here or in a follow-up. (In reply to comment #2) > 1: The format toolbar buttons have no label text (and we don't want them to > have label text). We don't? Why? If the user decides to go with text labels, shouldn't that s/he be able to achieve that? Or am I getting you wrong? > The icons also only comes in one size (anything else would of > course be strange) so the choices for "Settings for this toolbar" doesn't make > sense here. I wonder whether any choice we make here affects all themes (i.e. whether there's only one switch, so if we disabled it for Classic and Modern, would third-party themes be able to provide bigger icons?). More generally speaking, as long as selecting another option doesn't completely break the display, I'd be OK if we allowed selecting a size that we don't actually provide icons for in the bundled themes. > 2: When switching between modes (normal, html tags, html source, preview), > your choice of showing/hiding the format toolbar doesn't persist True. > we should probably treat all modes as they have the same > toolbar ("persisting through modes")? Need then to consider the fact that the > source mode view then would have a disabled format toolbar... :-/ Actually I'd support that idea. I see that the content is needlessly jumping around because of the showing and hiding when switching modes. From my POV it's not a blocker, though, i.e. you could do that in a follow-up or even leave it like it is. > 3: Showing/hiding a toolbar by selecting/deselecting in the context menu > doesn't update the menuitem in the View menu. Doing it from the View Menu > updates the context menuitems, though Good catch. This indeed needs to be fixed. > 4: When you have the customize sheet in front of you, it's possible to type in > the editable area Hmm, isn't that actually a feature? ;-) No seriously, does this have any negative implications? Note: I skimmed through the patch, but didn't even attempt to do a code review. Ratty might be a good candidate. Thanks for working on this!

(In reply to comment #3) > > 1: The format toolbar buttons have no label text (and we don't want them to > > have label text). > > We don't? Why? If the user decides to go with text labels, shouldn't that s/he > be able to achieve that? Or am I getting you wrong? I don't think the text will fit into the current toolbar... That is, we don't have any text labels and we (imo) don't want any. > > > The icons also only comes in one size (anything else would of > > course be strange) so the choices for "Settings for this toolbar" doesn't make > > sense here. > > I wonder whether any choice we make here affects all themes (i.e. whether > there's only one switch, so if we disabled it for Classic and Modern, would > third-party themes be able to provide bigger icons?). More generally speaking, > as long as selecting another option doesn't completely break the display, I'd > be OK if we allowed selecting a size that we don't actually provide icons for > in the bundled themes. Yes, but why would you want to give users an option that doesn't have any effect? > > 4: When you have the customize sheet in front of you, it's possible to type in > > the editable area > > Hmm, isn't that actually a feature? ;-) No seriously, does this have any > negative implications? It's a bit weird that you can type behind a pane since when you're customizing all menus are disabled - you don't really want a user do do anything but customizing (and we don't do that in mailNews iirc).

So, where are we regarding this? I think we should have it in a beta, so requesting status2.1. As for "Settings for this toolbar", I think we could just leave that one for now as it doesn't break anything if we have/keep it. If someone really wants the labels (with or without icons) despite the waste of space (think users with visual disabilities), I think they should be able to activate it. And bigger icons, if the theme provides them (again I don't think it matters that we don't have some in the default themes!). As for the ability to type while customizing: I'll leave that to Stefan and Ian; I would think that such nits could go to a follow-up, but then that's just my opinion.

I'm going to be trying to split the patches into easily reviewable patches, so this is the first one. This patch: * Moves some common commandset overlays into editorOverlay. * Moves a common broadcaster into editorOverlay. * Moves a singular use broadcaster out of editorOverlay into editor. * Renames the Editor/Compose toolbars to have a common name so that code can be shared and moved into editorOverlay.

Comment on attachment 514376 [details] [diff] [review] Tidy up some broadcasters and commandsets I'm not sure that changing the toolbar's id is a good idea. The rest looks OK.

(In reply to comment #8) > Comment on attachment 514376 [details] [diff] [review] > Tidy up some broadcasters and commandsets > > I'm not sure that changing the toolbar's id is a good idea. The rest looks OK. To effectively share code those ids need to be the same across the 3 windows (editor, text editor and message compose), unless you know another way of doing it.

Changes since last version: * No renaming of toolbar ids and keeping separate <command>s for editor vs message compose toggling.

Comment on attachment 514671 [details] [diff] [review] Tidy up some broadcasters and commandsets v2 >+ <broadcasterset id="editorBroadcasters"> > <broadcaster id="Communicator:WorkMode"/> >+ <broadcaster id="args" value="about:blank"/> >+ >+ <!-- view menu --> >+ <command id="cmd_viewEditModeToolbar" >+ oncommand="goToggleToolbar('EditModeToolbar','cmd_viewEditModeToolbar');" >+ checked="true"/> [Huh, I wonder why this command lives in a broadcasterset...] >+ <commandset id="tasksCommands"> >+ <commandset id="globalEditMenuItems"/> >+ <commandset id="selectEditMenuItems"/> >+ <commandset id="undoEditMenuItems"/> >+ <commandset id="clipboardEditMenuItems"/> >+ <command id="toggleSidebar"/> >+ </commandset> I think it would be neater to overlay editorCommands instead. > <!ENTITY % editorDTD SYSTEM "chrome://editor/locale/editor.dtd"> > %editorDTD; >+<!ENTITY % editorOverlayDTD SYSTEM "chrome://editor/locale/editorOverlay.dtd"> >+%editorOverlayDTD; IMHO it would be better to move the string into editor.dtd (given that the overlay doesn't use the string...) >-<broadcasterset id="composeBroadcasters"> >+<broadcasterset id="editorBroadcasters"> This change doesn't look right, since it's in compose.xul, not editor.xul - unless you can explain why, of course...

(In reply to comment #11) > Comment on attachment 514671 [details] [diff] [review] > Tidy up some broadcasters and commandsets v2 > > >+ <broadcasterset id="editorBroadcasters"> > > <broadcaster id="Communicator:WorkMode"/> > >+ <broadcaster id="args" value="about:blank"/> > >+ > >+ <!-- view menu --> > >+ <command id="cmd_viewEditModeToolbar" > >+ oncommand="goToggleToolbar('EditModeToolbar','cmd_viewEditModeToolbar');" > >+ checked="true"/> > [Huh, I wonder why this command lives in a broadcasterset...] There are a whole load that live in a broadcasterset in editorOverlay.xul > >-<broadcasterset id="composeBroadcasters"> > >+<broadcasterset id="editorBroadcasters"> > This change doesn't look right, since it's in compose.xul, not editor.xul - > unless you can explain why, of course... Well editorOverlay.xul is providing an overlay for the broadcasterset, I could have left them with an id of "broadcasterset" I suppose...

Changes since v2: * Moved commands from broadcasterset to commandset. * Overlay using editorCommands rather than tasksCommands. * Moved entity from editorOverlay.dtd to editor.dtd. * Changed id for broadcastersets.

One change I missed but I have fixed locally, in messengercompose.xul changed broadcaster_throbber to Editor:Throbber in the navigator-throbber button observes element.

This patch: * Turns menulist and stack elements in toolbar's children into toolbaritems * Adds missing classes to toolbaritems/buttons. * Minor tidy ups.

Changes since v1.0: * Also included changes made to editorSmileyOverlay.xul

Comment on attachment 515407 [details] [diff] [review] toolbar item tidy up v1.1 >-function EditorSelectFontSize() I don't think it's safe to change this because it's shared code (the menulist itself is OK because Thunderbird have forked editorOverlay.xul). [There's actually a better way of doing this by setting the value attribute on each of the menuitems so that you can simply read the menulist value.] >+ <toolbarbutton id="AlignPopupButton" >+ class="toolbarbutton-1" Why the class? >+ <toolbaritem id="paragraph-select-container" >+ class="chromeclass-toolbar-additional" >+ tooltiptext="&ParagraphSelect.tooltip;" >+ observes="cmd_renderedHTMLEnabler"> >+ <menulist id="ParagraphSelect" >+ class="toolbar-focustarget" >+ crop="right" >+ observes="paragraph-select-container"> Not sure this will work. (Not even sure what it's trying to do.) > </menulist> >+ </toolbaritem> I assume the indentation will be sorted out at some point?

(In reply to comment #17) > Comment on attachment 515407 [details] [diff] [review] > toolbar item tidy up v1.1 > > >+ <toolbarbutton id="AlignPopupButton" > >+ class="toolbarbutton-1" > Why the class? I have another patch coming up that switches the style sheet from using the toolbar id to using the toolbarbutton/item class. > > >+ <toolbaritem id="paragraph-select-container" > >+ class="chromeclass-toolbar-additional" > >+ tooltiptext="&ParagraphSelect.tooltip;" > >+ observes="cmd_renderedHTMLEnabler"> > >+ <menulist id="ParagraphSelect" > >+ class="toolbar-focustarget" > >+ crop="right" > >+ observes="paragraph-select-container"> > Not sure this will work. (Not even sure what it's trying to do.) We use it elsewhere for keeping the disable attribute in sync. > > > </menulist> > >+ </toolbaritem> > I assume the indentation will be sorted out at some point? I could, but I was trying to avoid a lot of whitespace change and taking blame of massive sections of code.

(In reply to comment #18) > (In reply to comment #17) > > Comment on attachment 515407 [details] [diff] [review] > > toolbar item tidy up v1.1 > > > > >+ <toolbarbutton id="AlignPopupButton" > > >+ class="toolbarbutton-1" > > Why the class? > I have another patch coming up that switches the style sheet from using the > toolbar id to using the toolbarbutton/item class. It will look the same, right?

(In reply to comment #19) > (In reply to comment #18) > > (In reply to comment #17) > > > Comment on attachment 515407 [details] [diff] [review] > > > toolbar item tidy up v1.1 > > > > > > >+ <toolbarbutton id="AlignPopupButton" > > > >+ class="toolbarbutton-1" > > > Why the class? > > I have another patch coming up that switches the style sheet from using the > > toolbar id to using the toolbarbutton/item class. > > It will look the same, right? That is the intention, yes.

Changes since v1.1: * Removed FontSize changes - will spin those off to another bug. * Changed class for AlignPopup, InsertPopup and absolutePostion buttons.

This patch (when applied on top of the toolbar item tidy up patch): * Switched the style sheets to use class rather than toolbar id to determine style.

(In reply to comment #18) > (In reply to comment #17) > > (From update of attachment 515407 [details] [diff] [review]) > > >+ <toolbarbutton id="AlignPopupButton" > > >+ class="toolbarbutton-1" > > Why the class? > I have another patch coming up that switches the style sheet from using the > toolbar id to using the toolbarbutton/item class. OK, but it was a really bad choice of class. (The latest patch is still a bad choice of class, since it's intended for use with chrome flags. So you should really think up a new class name.) > > >+ <toolbaritem id="paragraph-select-container" > > >+ class="chromeclass-toolbar-additional" > > >+ tooltiptext="&ParagraphSelect.tooltip;" > > >+ observes="cmd_renderedHTMLEnabler"> > > >+ <menulist id="ParagraphSelect" > > >+ class="toolbar-focustarget" > > >+ crop="right" > > >+ observes="paragraph-select-container"> > > Not sure this will work. (Not even sure what it's trying to do.) > We use it elsewhere for keeping the disable attribute in sync. OK, but you should then avoid putting any attributes on the toolbaritem, since the observes will copy those too (particularly bad in the case of the toolbar-focustarget class). > > > </menulist> > > >+ </toolbaritem> > > I assume the indentation will be sorted out at some point? > I could, but I was trying to avoid a lot of whitespace change and taking blame > of massive sections of code. In that case, would it be better putting the <toolbaritem> at the "wrong" indentation too, so at least that block is internally consistent?

Comment on attachment 515415 [details] [diff] [review] Switch from toolbar id to class patch v1.0 > #FormatToolbar > toolbarbutton > .toolbarbutton-text { > display: none; > } So, are you planning on giving the buttons text and setting the format toolbar to default to icons only?

(In reply to comment #23) > (In reply to comment #18) > > (In reply to comment #17) > > > (From update of attachment 515407 [details] [diff] [review]) > > > >+ <toolbarbutton id="AlignPopupButton" > > > >+ class="toolbarbutton-1" > > > Why the class? > > I have another patch coming up that switches the style sheet from using the > > toolbar id to using the toolbarbutton/item class. > OK, but it was a really bad choice of class. (The latest patch is still a bad > choice of class, since it's intended for use with chrome flags. So you should > really think up a new class name.) editorPrimaryToolbar.css already uses toolbarbutton-1 to assign a list-style-image or are you saying this is wrong?

(In reply to comment #24) > Comment on attachment 515415 [details] [diff] [review] > Switch from toolbar id to class patch v1.0 > > > #FormatToolbar > toolbarbutton > .toolbarbutton-text { > > display: none; > > } > So, are you planning on giving the buttons text and setting the format toolbar > to default to icons only? I plan on changing the above with the actual customisation patch.

Changes since toolbar item tidy up v1.2: * Added new class "format-bar-class" for use with toolbarbuttons/items typically found on the format toolbar. * Changed start indentation vele for new toolbaritems so the block is internally consistent. * Changed observe lines to observe specific attribute.

Changes since Switch from toolbar id to class patch v1.0: * Changed class used to "format-bar-class"

(In reply to comment #25) > (In reply to comment #23) > > (In reply to comment #18) > > > (In reply to comment #17) > > > > (From update of attachment 515407 [details] [diff] [review]) > > > > >+ <toolbarbutton id="AlignPopupButton" > > > > >+ class="toolbarbutton-1" > > > > Why the class? > > > I have another patch coming up that switches the style sheet from using the > > > toolbar id to using the toolbarbutton/item class. > > OK, but it was a really bad choice of class. (The latest patch is still a bad > > choice of class, since it's intended for use with chrome flags. So you should > > really think up a new class name.) > editorPrimaryToolbar.css already uses toolbarbutton-1 to assign a > list-style-image or are you saying this is wrong? That's different, since those are primary toolbar buttons... the ones that have traditionally been the large images above text. Although eventually I guess we could get rid of that class, since all toolbarbuttons will work the same in the end, the only difference being the toolbar modes. (In reply to comment #26) > (In reply to comment #24) > > (From update of attachment 515415 [details] [diff] [review]) > > > #FormatToolbar > toolbarbutton > .toolbarbutton-text { > > > display: none; > > > } > > So, are you planning on giving the buttons text and setting the format toolbar > > to default to icons only? > I plan on changing the above with the actual customisation patch. Right, it was just helpful for you to clarify that.

Comment on attachment 515534 [details] [diff] [review] Switch from toolbar id to class patch v1.1 >+toolbarbutton.format-bar-class { "-class" from the Department of Redundancy department :-) Mind you, my original suggestion was toolbarbutton.formatting-button ;-)

Comment on attachment 515533 [details] [diff] [review] toolbar item tidy up v1.3 >+ class="chromeclass-toolbar-additional format-bar-class" OK I'm confused. Why do you need the chromeclass-toolbar-additional class?

Comment on attachment 515533 [details] [diff] [review] toolbar item tidy up v1.3 r=me if you remove the chromeclass-additional-toolbar class. (I'll let you decide what you want to do with the format-bar-class class.)

(In reply to comment #13) > Created attachment 515238 [details] [diff] [review] > Tidy up some broadcasters and commandsets v2.1 > > Changes since v2: > * Moved commands from broadcasterset to commandset. > * Overlay using editorCommands rather than tasksCommands. > * Moved entity from editorOverlay.dtd to editor.dtd. > * Changed id for broadcastersets. I've noticed the debugQATextEditor.xul uses <commands> rather than <commandset>, is it best to change that?

Changes since toolbar item tidy up v1.3: * Removed chromeclass-additional-toolbar class. * Renamed format-bar-class class to formatting-button.

Changes since Switch from toolbar id to class patch v1.1: * Renamed class format-bar-class to formatting-button.

Comment on attachment 515238 [details] [diff] [review] Tidy up some broadcasters and commandsets v2.1 >+ <broadcasterset id="editorOverlayBroadcasters"> >+ <broadcaster id="Editor:Throbber" busy="false"/> > </broadcasterset> Not sure it's worth sharing one broadcaster. Also I would just call the set editorBroadcasters (compare nav/mailBroadcasters). >- <command id="cmd_showComposeToolbar" oncommand="goDoCommand('cmd_showComposeToolbar')"/> >- <command id="cmd_showFormatToolbar" oncommand="goDoCommand('cmd_showFormatToolbar')"/> >- <command id="toggleSidebar"/> You didn't remove the code for these. >-<broadcasterset id="composeBroadcasters"> >- <broadcaster id="Editor:Throbber" busy="false"/> >+<!-- broadcaster nodes are appended here from the overlays --> >+<broadcasterset id="editorOverlayBroadcasters"> I don't think Editor:Throbber belongs in this window. Mind you, the compose throbber doesn't seem to be hooked up to the progress listener, either directly or via the (missing) broadcaster_throbber element. > <menuitem id="menu_showComposeToolbar" > type="checkbox" > label="&showComposeToolbarCmd.label;" > accesskey="&showComposeToolbarCmd.accesskey;" >- command="cmd_showComposeToolbar" >- checked="true"/> >+ command="cmd_viewComposeToolbar"/> > <menuitem id="menu_showFormatToolbar" > type="checkbox" > label="&showFormatToolbarCmd.label;" > accesskey="&showFormatToolbarCmd.accesskey;" >- command="cmd_showFormatToolbar" >- checked="true"/> >+ command="cmd_viewFormatToolbar"/> For a moment I thought this was going to be a problem with persistence, but I think it's only a niggle for someone with a hidden toolbar (the checkbox will reappear until the toolbar is toggled twice.)

(In reply to comment #36) > Mind you, the compose throbber doesn't seem to be hooked up to the progress > listener, either directly or via the (missing) broadcaster_throbber element. Filed bug 637630.

Comment on attachment 515748 [details] [diff] [review] Switch from toolbar id to class patch v1.2 [Checked in: Comment 44] jftr, I will try to get to this in 1-2 days from now

Changes since Tidy up some broadcasters and commandsets v2.1: * Removed Editor:Throbber broadcaster from overlay. * Removed js code relating to cmd_showComposeToolbar and cmd_showFormatToolbar. * Gave broadcastersets different ids. * Changed from commands to commandsets in debugQATextEditorShell.xul.

Comment on attachment 516079 [details] [diff] [review] Tidy up some broadcasters and commandsets v2.2 [Checked in: Comment 42] Note to self: make -C suite/debugQA chrome doens't work.

Comment on attachment 515748 [details] [diff] [review] Switch from toolbar id to class patch v1.2 [Checked in: Comment 44] +toolbarbutton.formatting-button { Do you have .formatting-button's that are not toolbarbuttons (not my call really, just curious on why you need the extra selector)?

Comment on attachment 516079 [details] [diff] [review] Tidy up some broadcasters and commandsets v2.2 [Checked in: Comment 42] http://hg.mozilla.org/comm-central/rev/369ad40b3fc1

Comment on attachment 515745 [details] [diff] [review] toolbar item tidy up v1.4 [Checked in: Comment 43] http://hg.mozilla.org/comm-central/rev/a4afd59a66b2

Comment on attachment 515748 [details] [diff] [review] Switch from toolbar id to class patch v1.2 [Checked in: Comment 44] http://hg.mozilla.org/comm-central/rev/118029396d1b

(In reply to comment #41) > Comment on attachment 515748 [details] [diff] [review] > Switch from toolbar id to class patch v1.2 [Checked in: Comment 44] > > +toolbarbutton.formatting-button { > > Do you have .formatting-button's that are not toolbarbuttons (not my call > really, just curious on why you need the extra selector)? There are toolbaritems which have the formatting-button class, but neither of our themes style them, but that doesn't mean a theme out there won't want to.

Comment on attachment 515745 [details] [diff] [review] toolbar item tidy up v1.4 [Checked in: Comment 43] >diff --git a/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd b/editor/ui/locales/en-US/chrome/composer/editorOverlay.dtd >-<!ENTITY alignjustifyButton.tooltip "Align text along left and right margins"> >+<!ENTITY alignJustifyButton.tooltip "Align text along left and right margins"> Please be careful changing editor/ - this change broke the Thunderbird tree (MozMill tests) as this caused a missing string in our version of editorOverlay.xul. (I'd also query the real benefit of changing the string name like this, as although it gives a prettier name, it also causes more work for l10n). I fixed the Thunderbird tree with this: http://hg.mozilla.org/comm-central/rev/c2ea4943b532

When I was doing the other css changes for switching from toolbar id to class, it looks like I missed one.

Comment on attachment 517045 [details] [diff] [review] Fix css for formatting-button menulist v1.0 [Checked in: Comment 49] >+.formatting-button menulist:-moz-lwtheme { Please can you insert a > as we know the menulist will always be a direct child of the toolbaritem in this case. r=me with that fixed.

Comment on attachment 517045 [details] [diff] [review] Fix css for formatting-button menulist v1.0 [Checked in: Comment 49] Checked in with > added. http://hg.mozilla.org/comm-central/rev/7c2bd99a597d

clearing status request; IanN if this is close to being ready and has no remaining l10n impact, feel free to re-request.

Comment on attachment 9139207 [details] [diff] [review] Make EditToolbar customisable [Checked in: Comment 54] Target 2.74a1 2.53.3 LGTM Previous comments from version 0.1 seem all the be addressed. A few notes/NITs: > Lets small icons be used in Composer / Text Editor for Classic Theme In Modern gives you a small SeaMonkey throbber and normal composer icons. Add small Modern icons in a follow-up I think. > editor/ui/composer/content/editorOverlay.xul > - <command id="cmd_viewCompToolbar" oncommand="goToggleToolbar('EditToolbar','cmd_viewCompToolbar');" checked="true"/> While we touch the code I would reformat and line up vertically. Edit 28.04. Given that it was a removal ignore and lets leave the line below alone. Formatting in the XUL files in now inconsistent but you probably know and want to prerserve "BLAME". This would need a full reformat and can be done later when the codebase in better shape.

Pushed by frgrahl@gmx.net: https://hg.mozilla.org/comm-central/rev/6da7d36ce03e Allow customization of toolbars in Composer and formatting toolbar in MailNews Composition - EditToolbar part. r=frg DONTBUILD

Comment on attachment 9144499 [details] [diff] [review] Make FormatToolbar customisable As discussed. Error with plaintext repliesForwards: >Timestamp: 5/3/2020, 6:20:39 PM >Error: document.getElementById(...) is null >Source File: chrome://messenger/content/messengercompose/MsgComposeCommands.js Line: 1312

Comment on attachment 9145311 [details] [diff] [review] Make FormatToolbar customisable v1.1 [Checked in: Comment 58] Target 2.75a1 2.53.3 LGTM. Just be aware that this introduces a possible problem. The user can move formatting toolbar items to the mail toolbar and these will not be disabled/hidden then. I think we can live with it and can look into it in a follow-up bug.

Pushed by frgrahl@gmx.net: https://hg.mozilla.org/comm-central/rev/e66503accfba Allow customization of toolbars in Composer and formatting toolbar in MailNews Composition - formatting toolbar part. r=frg DONTBUILD