Closed Bug 1313288 Opened 4 years ago Closed 4 years ago

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

Categories

(SeaMonkey :: MailNews: Composition, defect)

SeaMonkey 2.49 Branch
defect
Not set
normal

Tracking

(seamonkey2.48 unaffected, seamonkey2.49esr fixed)

RESOLVED FIXED
seamonkey2.49
Tracking Status
seamonkey2.48 --- unaffected
seamonkey2.49esr --- fixed

People

(Reporter: RainerBielefeldNG, Assigned: frg)

References

Details

Attachments

(4 files, 2 obsolete files)

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.
a) also other Icons and menu functions of Email Client do not work
maybe related to bug 1313041 ?? (I have not tested all of the composer keys, buttons and access keys in bug 1313041 )
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [easyconfirm]
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.
Flags: needinfo?(philip.chee)
Flags: needinfo?(iann_bugzilla)
(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. :)
Attached file 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.
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.
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
Attached patch 1313288-wip.patch (obsolete) — Splinter Review
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.
See Also: → 917545
(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.
Flags: needinfo?(richard.marti)
(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.
Flags: needinfo?(richard.marti)
See Also: → 1314457
I put a patch for Chatzilla in bug 1314457. Maybe someone else can check it out too. Seems to work.
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 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.
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.
(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.
>> 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.
Attached patch 1313288-remove-obsolete.patch (obsolete) — Splinter Review
Compile is running. This one should be reviewable if it works.
Assignee: nobody → frgrahl
Attachment #8806365 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Comment on attachment 8807614 [details] [diff] [review]
1313288-remove-obsolete.patch

Looks good to me.
Flags: needinfo?(philip.chee)
Flags: needinfo?(iann_bugzilla)
Attachment #8807614 - Flags: review?(philip.chee)
Attachment #8807614 - Flags: review?(iann_bugzilla)
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
Attachment #8807614 - Flags: review?(philip.chee) → review+
Oops. See Also: bug 1313039 comment 4
The lazy getter seems to work. Review+ from Philip Chee carried forward.

Will put up a separate patch for the editor.
Attachment #8807614 - Attachment is obsolete: true
Attachment #8807614 - Flags: review?(iann_bugzilla)
Attachment #8807850 - Flags: review+
> 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.
Attachment #8807851 - Flags: review?(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.
Attachment #8807851 - Flags: review?(philip.chee) → review+
Added the import:

https://hg.mozilla.org/comm-central/rev/6a6efc993defce014a89e080364292327385a114
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.49
You need to log in before you can comment on or make changes to this bug.