1442045 - macwindowmenu.inc removal

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, enhancement)

Description

[Brendan Dahl [:bdahl]]

Over in bug 1441378 we're planning to remove macwindowmenu.inc. You'll probably want to fork the file or inline it like we did in Firefox.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: helpIncludes.patch

Philipp, I'm asking you for r? because you the whole overlay mechanism very good and this patch has also some Mac specialties.

Instead of overlays (baseMenuOverlay.xul and macMenuOverlay.xul) I'm using now includes (helpMenu.inc and macWindowMenu.inc).

The viewSourceOverlay.xul is a bit special with the help menu as I think this window doesn't need all help item like "Restart with Add-ons disabled" etc. I've added only the "TB Help Contents".

Comment 2

[Richard Marti (:Paenglab)]

Bug 1441378 is in autoland and will bust us because of the removal of toolkit/content/macWindowMenu.inc.

Jörg, maybe you can look at this patch and Philipp does a post review on Mac. All I can say is, it works on Mac too.

Comment 3

[Jorg K (CEST = GMT+2)]

Comment on attachment 8955310 [details] [diff] [review]
helpIncludes.patch

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

I'll try it now to get it landed before 12:00 PM.

::: mail/base/content/mailWindowOverlay.xul
@@ +54,5 @@
>  <script type="application/javascript" src="chrome://global/content/printUtils.js"/>
>  <script type="application/javascript" src="chrome://messenger/content/msgViewPickerOverlay.js"/>
>  <script type="application/javascript" src="chrome://messenger/content/plugins.js"/>
>  <script type="application/javascript" src="chrome://global/content/viewZoomOverlay.js"/>
> +<script type="application/javascript" src="chrome://communicator/content/utilityOverlay.js"/>

What's "communicator" doing here?

Comment 4

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+1) from comment #3)
> Comment on attachment 8955310 [details] [diff] [review]
> helpIncludes.patch
> 
> Review of attachment 8955310 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'll try it now to get it landed before 12:00 PM.
> 
> ::: mail/base/content/mailWindowOverlay.xul
> @@ +54,5 @@
> >  <script type="application/javascript" src="chrome://global/content/printUtils.js"/>
> >  <script type="application/javascript" src="chrome://messenger/content/msgViewPickerOverlay.js"/>
> >  <script type="application/javascript" src="chrome://messenger/content/plugins.js"/>
> >  <script type="application/javascript" src="chrome://global/content/viewZoomOverlay.js"/>
> > +<script type="application/javascript" src="chrome://communicator/content/utilityOverlay.js"/>
> 
> What's "communicator" doing here?

It was removed from here: https://dxr.mozilla.org/comm-central/source/mail/base/content/baseMenuOverlay.xul#16 and needs to be added to work. utilityOverlay.js is placed from mail to communicator here: https://dxr.mozilla.org/comm-central/source/mail/base/jar.mn#141

Comment 5

[Jorg K (CEST = GMT+2)]

Hmm, Windows is still working without the patch, so that must only affect Mac.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bf7adc1ba8ee
Replace baseMenuOverlay.xul, macMenuOverlay.xul and macWindowMenu.inc with inlining and preprocessing. rs=jorgk

Comment 7

[Richard Marti (:Paenglab)]

Yes it affects only Mac because of the removal of macwindowmenu.inc. The other parts are to remove the overlays.

Comment 8

[Jorg K (CEST = GMT+2)]

My build was so slow that I didn't get to test before pushing. So I had to trust Swiss quality. Now my build is finished and it still appears to be working on Windows, although I'm not sure where to look. I checked the help menus on the main window, compose window, address book window and view source window and that one doesn't appear to work, it only shows "Help Contents". Looks like we need a follow-up here.

Comment 9

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+1) from comment #8)
> My build was so slow that I didn't get to test before pushing. So I had to
> trust Swiss quality. Now my build is finished and it still appears to be
> working on Windows, although I'm not sure where to look. I checked the help
> menus on the main window, compose window, address book window and view
> source window and that one doesn't appear to work, it only shows "Help
> Contents". Looks like we need a follow-up here.

Iwrote in comment #1:
> 
> The viewSourceOverlay.xul is a bit special with the help menu as I think
> this window doesn't need all help item like "Restart with Add-ons disabled"
> etc. I've added only the "TB Help Contents".

Do we really need release notes etc. in this window? It's not a main window and all what would be needed is this item. Or am I wrong?

Comment 10

[Jorg K (CEST = GMT+2)]

Sorry, I missed that comment. How about removing the Help menu from that window altogether?

Comment 11

[Richard Marti (:Paenglab)]

TI think this help menu entry can stay. Then the user can use it when he searches something the source shows. And on Mac then the menu search field is also accessible.