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)
Thunderbird
General
Tracking
(thunderbird40 wontfix, thunderbird41 wontfix, thunderbird42 wontfix, thunderbird43 fixed, thunderbird_esr38+ wontfix)
People
(Reporter: jorgk-bmo, Unassigned)
References
Details
(Whiteboard: [tb-papercut])
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
+++ 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.
Reporter | ||
Comment 1•9 years ago
|
||
TB 43 will be fixed when the patch lands today.
status-thunderbird40:
--- → wontfix
status-thunderbird41:
--- → wontfix
status-thunderbird42:
--- → wontfix
status-thunderbird43:
--- → fixed
status-thunderbird_esr38:
--- → affected
tracking-thunderbird_esr38:
--- → ?
Reporter | ||
Comment 2•9 years ago
|
||
This is the changeset that needs landing on the TB 38 release branch:
https://hg.mozilla.org/mozilla-central/rev/af909b481b95
Comment 3•9 years ago
|
||
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).
Reporter | ||
Comment 4•9 years ago
|
||
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.
Comment 5•9 years ago
|
||
(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 :)
Updated•9 years ago
|
Keywords: checkin-needed
Comment 6•9 years ago
|
||
No patch to check in?!
Reporter | ||
Comment 7•9 years ago
|
||
Hmm, no, it's a M-C patch. The "checkin-needed" got cloned from the other bug and I forgot to clear it. Sorry!
Updated•9 years ago
|
Reporter | ||
Comment 8•9 years ago
|
||
The fix in bug 772796 caused a regression, see bug 1206483 for details.
Reporter | ||
Comment 9•9 years ago
|
||
The fix in bug 772796 got backed out and landed again with slight modifications:
https://hg.mozilla.org/mozilla-central/rev/0ff44857e33f
Comment 10•9 years ago
|
||
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.
Reporter | ||
Comment 11•9 years ago
|
||
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 ;-)
Comment 12•9 years ago
|
||
This is the download link for the current file:
https://hg.mozilla.org/releases/mozilla-esr38/raw-file/tip/editor/libeditor/nsHTMLEditRules.cpp
Reporter | ||
Comment 13•9 years ago
|
||
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.
Comment 14•6 years ago
|
||
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.
Reporter | ||
Comment 15•6 years ago
|
||
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.
Description
•