Closed Bug 1203847 Opened 9 years ago Closed 6 years ago

Take "Bug 772796 - Joining <div> with <pre> obliterates preformatting" into TB 38.x

Categories

(Thunderbird :: General, defect)

defect
Not set
normal

Tracking

(thunderbird40 wontfix, thunderbird41 wontfix, thunderbird42 wontfix, thunderbird43 fixed, thunderbird_esr38+ wontfix)

RESOLVED WONTFIX
Tracking Status
thunderbird40 --- wontfix
thunderbird41 --- wontfix
thunderbird42 --- wontfix
thunderbird43 --- fixed
thunderbird_esr38 + wontfix

People

(Reporter: jorgk-bmo, Unassigned)

References

Details

(Whiteboard: [tb-papercut])

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #772796 +++ This is the much hated TB papercut (https://wiki.mozilla.org/Thunderbird/Papercuts): Summary of bug 772796 was changed: Original: Deleting quotes causes preformats to be deleted New: Joining <div> with <pre> obliterates preformatting This has now been fixed in the M-C editor and could be included in TB 38.x at some stage.
TB 43 will be fixed when the patch lands today.
This is the changeset that needs landing on the TB 38 release branch: https://hg.mozilla.org/mozilla-central/rev/af909b481b95
I think that a likely target for this is Thunderbird 38.4.0, not the upcoming 38.3.0, which would give time for a full beta cycle. Unfortunately we do not currently have the equivalent of THUNDERBIRD_38_VERBRANCH available for the beta builds. Perhaps we could add one, but that results in a certain permanent overhead (we have to manually merge to that branch before each beta build).
The patch could ride the trains a bit in Firefox, but sadly, there little <pre> editing takes place. It's the TB composition window that exercises all the dark corners of the M-C editor. I've just checked that the M-C patch fixes the TB papercut as described in bug 772796 comment #7 just beautifully like it was before May 2010!! Anyway, we've waited five years for a fix, so if we wait six weeks more and have it come out in 38.4, that's still OK. However, additional wait time won't provide additional testing. Also, Kent, you're becoming risk-adverse ;-) In TB 38.0 we included M-C changes hot of the press.
(In reply to Jorg K (GMT+2) from comment #4) > Also, Kent, you're becoming risk-adverse ;-) In TB 38.0 we included M-C > changes hot of the press. That's because in the initial phases of the release, we had throttled updates, and were solving serious regression issues. Also, Wayne has now learned to regularly remind me of risk to throttle my aggressive tendencies :)
Keywords: checkin-needed
No patch to check in?!
Hmm, no, it's a M-C patch. The "checkin-needed" got cloned from the other bug and I forgot to clear it. Sorry!
The fix in bug 772796 caused a regression, see bug 1206483 for details.
The fix in bug 772796 got backed out and landed again with slight modifications: https://hg.mozilla.org/mozilla-central/rev/0ff44857e33f
I'm afraid that the landed patch in bug 772796 does not apply cleanly to esr38, and the changes needed are not trivial. So this needs more work before we can try to get it into esr38.
Really? The code change in the patch (attachment 8664172 [details] [diff] [review]) consists of a one line change TouchContent::no); changed to TouchContent::yes); and the insertion of two blocks of code. That must be easily applyable. If you send me nsHTMLEditRules.cpp as of ESR 38, I can adjust the patch. But I'm sure, so can you ;-)
Attached patch Patch adapted for TB 38 (deleted) — Splinter Review
Hmm, some 'busy refactoring' has gone on there. I adapted the patch to the best of my abilities without compiling or testing. Here is a diff, not a proper patch, sorry.
Is this still an issue? Some time has passed, and build / tree configurations change around shared code. I can't repro in a fresh build.
Well, looks like Kent had problems applying my patch to mozilla-esr38 for inclusion into TB 38. Since TB 60.2.1 was released yesterday, we can just close the bug.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: