1313288 - In Email Composer Window Icons 'Send', 'Address', 'Attach', 'Spell' do not work

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Composition, defect)

Description

[Rainer Bielefeld]

Steps how to reproduce  NOT reproducible REPRODUCIBLE with official en-US SeaMonkey 2.49a1  (NT 6.1; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0 Build 20161026000046  (Default Classic Theme) on German WIN7 64bit:

0. Launch Email client
1. Click Compose-Icon
   » Email Composer opens
2. Add Recipient, minimum Subject and Body
3. Click one of the mentioned Icons
   Bug: Nothing happens.

Comment 1

[Rainer Bielefeld]

a) also other Icons and menu functions of Email Client do not work

Comment 2

[dominique fillon]

maybe related to bug 1313041 ?? (I have not tested all of the composer keys, buttons and access keys in bug 1313041 )

Comment 3

[Frank-Rainer Grahl (:frg)]

Related to Bug 1313039 which ports bug 1312143. I am quite sure SeaMonkey needs additional fixes or maybe even a fork of the removed parts.

Comment 4

[Frank-Rainer Grahl (:frg)]

Attachment: 1313288-hack.patch

If someone builds locally this should fix it for now.

Works but not a proper patch. If we decide to keep the parts they should probably be moved with history from m-c and end in mailnews if TB also needs them or probably under suite\common.

Comment 5

[Hartmut Figge]

(In reply to Frank-Rainer Grahl from comment #4)

> If someone builds locally this should fix it for now.

It compiles up to
Error: /home/hafi/moz-work/src/obj-x86_64-pc-linux-gnu/suite/installer/package-manifest:130: Missing file(s): bin/components/dom_telephony.xpt

but can be tested nevertheless. Well, it works. Even the horrors of yesterday, no structure at all in MailNews, are fixed. Nice work, Frank-Rainer. :)

Comment 6

[Hartmut Figge]

Attachment: Errors in packager

I'm not sure if this is related, but it may be. After eliminating dom_telephony.xpt from package-manifest.in I get the attached errors.

Comment 7

[Frank-Rainer Grahl (:frg)]

The dupes bug is tracked is tracked in bug 1313670. Just remove the 

>     if unexpected_dupes:
>         print "ERROR: The following duplicated files are not allowed:"
>         print "\n".join(unexpected_dupes)
>         sys.exit(1)

for now from find-dupes.py. See https://hg.mozilla.org/mozilla-central/rev/532415dbd620

Please apply the changes in bug 1313726 or you might have a problem with a missing sandboxing library.

That only leaves bug 1300547. Sorry no fix for now.

I digged a little further into this. The original breakage might be because Chatzilla uses some of the obsolete files. Not sure if we need/should keep them for other addons.

Comment 8

[Jorg K (CEST = GMT+2)]

I tried a "Daily" try build of TB (which I'm doing despite the bustage)
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f912cca79bb4d82348cc9a435f7127a0bd20652d
and "Send", "Spelling" and "Attach" work in TB. So this bug here must be SM specific.

BTW, great work by FRG in bug 1313858 and bug 1313860 to keep us going. "Daily" try build here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2b410485bd8e70afab57575f8478eb107e85ca0c

Comment 9

[Frank-Rainer Grahl (:frg)]

Attachment: 1313288-wip.patch

This fixes the fallout for SeaMonkey without copying the obsolete parts but leaves DOMi and Chatzilla broken. They would need to be fixed separately. 

The changes might affect other older addons too so just let me know which way is preferable.

Comment 10

[Frank-Rainer Grahl (:frg)]

Lightning is also using globaloverlay.xul:

https://dxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-alarm-dialog.xul

Comment 11

[Jorg K (CEST = GMT+2)]

(In reply to Frank-Rainer Grahl from comment #9)
> This fixes the fallout for SeaMonkey without copying the obsolete parts but
> leaves DOMi and Chatzilla broken. They would need to be fixed separately. 
For DOMi we have bug 1313906,

> Lightning is also using globaloverlay.xul
Richard, can you look into this.

Comment 12

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+1) from comment #11)
> (In reply to Frank-Rainer Grahl from comment #9)
> > Lightning is also using globaloverlay.xul
> Richard, can you look into this.

I must have missed that. I'll fix this in a new bug.

Comment 13

[Frank-Rainer Grahl (:frg)]

I put a patch for Chatzilla in bug 1314457. Maybe someone else can check it out too. Seems to work.

Comment 14

[Robert Kaiser]

I wonder if it would make more sense to load it in tasksOverlay or utilityOverlay from communicator/

That said, I believe another symptom of this is that marking emails read or deleting them from messenger doesn't work, and the right-click context menus are broken as well.

Comment 15

[Robert Kaiser]

Comment on attachment 8806365 [details] [diff] [review]
1313288-wip.patch

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

::: suite/browser/navigator.xul
@@ -42,5 @@
>          persist="screenX screenY width height sizemode">
>  
>    <!-- Generic Utility -->
> -  <script type="application/javascript" src="chrome://global/content/nsUserSettings.js"/>
> -  <script type="application/javascript" src="chrome://global/content/nsClipboard.js"/>

We need to see if their functions are actually used and either replace them with newer ways to do this (preferably) or re-import the obsolete files on our side.

::: suite/common/contentAreaContextOverlay.xul
@@ -10,5 @@
>    <script type="application/javascript">
>      // Global variable that holds the nsContextMenu instance.
>      var gContextMenu = null;
>    </script>
> -  <script type="application/javascript" src="chrome://global/content/inlineSpellCheckUI.js"/>

This one I see you inlined, which is nice.

Comment 16

[Frank-Rainer Grahl (:frg)]

With the wip patch the obsolete parts in the hack patch are no longer needed in SeaMonkey. I didn't have much time yesterday but a first glance dialogOverlay,strres, nsClipboard and nsUserSettings are not used. Same for the tooltip in globalOverlay.

I just replaces globalOverlay.xul with globalOverlay.js which is probably overkill as you noted.

DOMi and cz will be fixed too. The only question which remains is if the obsolete parts should be kept for other older addons which might need them.

Comment 17

[Robert Kaiser]

(In reply to Robert Kaiser from comment #14)
> That said, I believe another symptom of this is that marking emails read or
> deleting them from messenger doesn't work, and the right-click context menus
> are broken as well.

And I can confirm that the patch fixes those as well.

(In reply to Frank-Rainer Grahl from comment #16)
> The only question which remains is if the
> obsolete parts should be kept for other older addons which might need them.

I don't think so. Most add-ons probably don't use them anyhow and the others can be fixed up easily when the new versions get released.

Comment 18

[Frank-Rainer Grahl (:frg)]

>> I don't think so. Most add-ons probably don't use them anyhow and the others 
>> can be fixed up easily when the new versions get released.

Sounds good to me. Just wasn't sure because SeaMonkey users usually tend to run a batch of older add-ons and two of the bundled ones broke.

If this creates a problem later the obsolete parts could still be integrated into the next release if desired.

Comment 19

[Frank-Rainer Grahl (:frg)]

Attachment: 1313288-remove-obsolete.patch

Compile is running. This one should be reviewable if it works.

Comment 20

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8807614 [details] [diff] [review]
1313288-remove-obsolete.patch

Looks good to me.

Comment 21

[Philip Chee]

Comment on attachment 8807614 [details] [diff] [review]
1313288-remove-obsolete.patch

> +Components.utils.import("resource://gre/modules/InlineSpellChecker.jsm");
 
>  var gContextMenuContentData = null;
> +var InlineSpellCheckerUI = new InlineSpellChecker();

How about a lazy getter?
XPCOMUtils.defineLazyGetter(this, "InlineSpellCheckerUI", function() {
  var tmp = {};
  Components.utils.import("resource://gre/modules/InlineSpellChecker.jsm", tmp);
  return new tmp.InlineSpellChecker();
});

This needs to be fixed too :P
https://hg.mozilla.org/comm-central/annotate/d39b521c1bb7/editor/ui/dialogs/content/EdSpellCheck.js#l6

Comment 22

[Philip Chee]

Oops. See Also: bug 1313039 comment 4

Comment 23

[Frank-Rainer Grahl (:frg)]

Attachment: 1313288-remove-obsolete-V2.patch

The lazy getter seems to work. Review+ from Philip Chee carried forward.

Will put up a separate patch for the editor.

Comment 24

[Frank-Rainer Grahl (:frg)]

https://hg.mozilla.org/comm-central/rev/591634b0150581cdb65f6612ded8addf271a0593

Comment 25

[Frank-Rainer Grahl (:frg)]

Attachment: 1313288-editor.patch

> Without this, spellcheck still works as before.

I see the same behaviour as Richard in Bug 1313039. Not sure if this is needed. Spell check works in Composer and Mailnews without it as far as I see it.

Comment 26

[Philip Chee]

Comment on attachment 8807851 [details] [diff] [review]
1313288-editor.patch

To be on the safe side, also add:
Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
before using XPCOMUtils.

Comment 27

[Frank-Rainer Grahl (:frg)]

Added the import:

https://hg.mozilla.org/comm-central/rev/6a6efc993defce014a89e080364292327385a114