Closed Bug 1448946 Opened 5 years ago Closed 4 years ago

Remove remaining editor overlays


(Thunderbird :: General, enhancement)

Not set


(Not tracked)

Thunderbird 61.0


(Reporter: jorgk-bmo, Assigned: Paenglab)




(4 files)

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

This is a list, maybe incomplete:
It is incomplete, EdDialogOverlay.xul for example overlays over 20 files. The overlaying in editor is a mess.
Keywords: leave-open
Pushed by
remove trailing spaces in editor. rs=white-space-only
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 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?
Assignee: nobody → richard.marti
Attachment #8964117 - Flags: review?(jorgk)
Attachment #8964117 - Flags: review?(frgrahl)
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:

If there are failures, they come from the second patch as a try with only the first patch was successful ( ).
Attachment #8964118 - Flags: review?(jorgk)
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.
Attachment #8964117 - Flags: review?(jorgk) → review+
Comment on attachment 8964118 [details] [diff] [review]

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.
Attachment #8964118 - Flags: review?(jorgk) → review+
Keywords: leave-open
Comment on attachment 8964117 [details] [diff] [review]

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?
Attachment #8964117 - Flags: review?(frgrahl) → review+
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.
Keywords: checkin-needed
> So it's better to leave them like they are.

You could put a note mentioning the original file in the dtd.
Long discussion on this here:

We learned from our errors, renaming the files will cause retranslations and even the note could be overlooked.
Pushed by
Replace the Editor overlays with inlining and pre-processing. r=jorgk,frg
Replace editorOverlay.xul with inlining and pre-processing. r=jorgk
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 61.0
Attachment #8965266 - Flags: review?(jorgk)
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.
Attachment #8965266 - Flags: review?(jorgk) → review+
You need to log in before you can comment on or make changes to this bug.