1448946 - Remove remaining editor overlays

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

In bug 1448657 some easy unused ones were removed, that leaves the hard ones :-(

This is a list, maybe incomplete:
EdColorPropsOverlay.xul
EdImageOverlay.xul
EdImageOverlayOverlay.xul
mailComposeEditorOverlay.xul
EdSpellCheckOverlay.xul

Comment 1

[Richard Marti (:Paenglab)]

It is incomplete, EdDialogOverlay.xul for example overlays over 20 files. The overlaying in editor is a mess.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f466d0abed9f
remove trailing spaces in editor. rs=white-space-only

Comment 3

[Richard Marti (:Paenglab)]

Attachment: editorOverlay.patch

This patch removes all editor overlays TB uses. There are still overlays for SM which I haven't touched because I can't check them.

Some dialogs had the EdDialogOverlay.xul referenced althought they haven't used it. So I removed the overlay.

There where also some files which are packaged for TB but never used. I moved them  to SM in jar.mn. Thanfully Jörg removed the trailing white spaces in the editor files and you can see now what i changed in the files.

Frank-Rainer, please can you check that I haven't broken SM more that it is?

Comment 4

[Richard Marti (:Paenglab)]

Attachment: composeOverlay.patch

Composer had a overlay which added some menus and format buttons. Now the menus and commandsets/keysets directly to messengercompose.xul. The format buttons are in a include file.

I also removed some CSS which isn't used in TB and was forgotten to remove.

Try build with both patches: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1e3064ad1f6fdd2922a68c655826ae2435471a6c

If there are failures, they come from the second patch as a try with only the first patch was successful ( https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3f1d1101aaea0bfd75b6d7884f7cf0b800811f85 ).

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: editorOverlay-no-ws.patch

This might be a little easier to look at since the code in
mailComposeEditorOverlay.xul -> EdImageLinkLoader.js
doesn't show up as a huge difference.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 8964118 [details] [diff] [review]
composeOverlay.patch

This comment applies to both patches.

This is very hard to review, huge patches, lots of reshuffling. In part we need to rely on the patch author who has a lot of experience in this field. On the other hand, I tried to find a malfunction in the HTML editor and didn't find anything.

So thanks a lot for this HUGE effort!!!

Oh, funny that onAccept() from EdColorProps.js had three copies ;-)

How about landing this? With all due respect, I don't think FRG can give a detailed review here and I assume that SM's mail part is not working well enough to test this. I'd like to see this land soon since overlay support will be gone from M-C in a matter of days, perhaps a week.

Comment 7

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8964117 [details] [diff] [review]
editorOverlay.patch

This part is currently broken in SeaMonkey so I can't test it.

You renamed mailnews/compose/content/mailComposeEditorOverlay.xul to editor/ui/dialogs/content/EdImageLinkLoader.js but still keep 

> chrome://messenger/locale/messengercompose/mailComposeEditorOverlay.dtd

Shouldn't the dtd be renamed for consistency too?

Comment 8

[Richard Marti (:Paenglab)]

Thanks. Renaming the DTD files leads to create a complete new file for the localizers. So it's better to leave them like they are.

Comment 9

[Frank-Rainer Grahl (:frg)]

> So it's better to leave them like they are.

You could put a note mentioning the original file in the dtd.

Comment 10

[Jorg K (CEST = GMT+2)]

Long discussion on this here:
https://groups.google.com/d/msg/mozilla.dev.l10n/yCA_4qiJWGc/BIimkRiaAgAJ

We learned from our errors, renaming the files will cause retranslations and even the note could be overlooked.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1ff14a05b2a1
Replace the Editor overlays with inlining and pre-processing. r=jorgk,frg
https://hg.mozilla.org/comm-central/rev/3213fa235aef
Replace editorOverlay.xul with inlining and pre-processing. r=jorgk

Comment 12

[Onno Ekker [:nONoNonO UTC+1]]

Attachment: Remove trailing empty lines from editor/ui/ .dtd-files

Comment 13

[Jorg K (CEST = GMT+2)]

Comment on attachment 8965266 [details] [diff] [review]
Remove trailing empty lines from editor/ui/ .dtd-files

OK, thanks, maybe better landed as part of bug 1399756? I'll take care of it.