Closed Bug 1455903 Opened 6 years ago Closed 6 years ago

Fix CloneAttributesWithTransaction() (affecting Image insertion in TB and Mozmill test-image-insertion-dialog.js, test-multipart-related.js)

Categories

(Core :: DOM: Editor, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 --- fixed

People

(Reporter: jorgk-bmo, Assigned: masayuki)

References

Details

(Keywords: regression, Whiteboard: [Thunderbird-testfailure: Z all])

Attachments

(1 file)

Carried over from bug 1454842 comment #22.

The "multipart related" test inserts an image and that fails, hence the:
EXCEPTION: a != b: 'text/html' != 'multipart/related'.

So the message gets created as HTML only, no attached image.

M-C last good: 39ccabfd7d0712a45335325cb24b0e0b2b
M-C first bad: dd0e54d786743974a50a338059bcd68a09
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=39ccabfd7d0712a45335325cb24b0e0b2b&tochange=dd0e54d786743974a50a338059bcd68a09
Wow, I can't insert any image any more manually :-(

Masayuki-san, could that be related to your bug 1451672 which is in the regression interval?
Flags: needinfo?(masayuki)
Summary: Fix Mozmill: test-image-insertion-dialog.js, test-multipart-related.js → Image insertion broken: Fix Mozmill: test-image-insertion-dialog.js, test-multipart-related.js
Hmm, image gets inserted as <img> with all attributes missing :-(
Whiteboard: [Thunderbird-testfailure: Z all]
Confirmed via local backout that bug 1451672 caused this.
Blocks: 1451672
Keywords: regression
If it helps: The image is inserted here:
https://dxr.mozilla.org/comm-central/rev/8d636ad5810b5586433294c16cb2376d826fcca8/editor/ui/dialogs/content/EdInputImage.js#155
and then attributes are stored onto it a few lines later. Maybe adding the attributes fails now.
Thank you for the investigation and sorry for the regression.

According to comment 2, must be caused by this line:
https://searchfox.org/mozilla-central/rev/fcd5cb3515d0e06592bba42e378f1b9518497e3d/editor/libeditor/EditorBase.cpp#2573

I'll try to reproduce this with automated tests and fix this line soon.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Flags: needinfo?(masayuki)
Thanks for the quick action and the massive new test :-)
Component: Message Compose Window → Editor
Product: Thunderbird → Core
Summary: Image insertion broken: Fix Mozmill: test-image-insertion-dialog.js, test-multipart-related.js → Fix CloneAttributesWithTransaction() (affecting Image insertion in TB and Mozmill test-image-insertion-dialog.js, test-multipart-related.js)
Comment on attachment 8970403 [details]
Bug 1455903 - Make EditorBase::CloneAttributesWithTransaction() set sourceElement to aSourceElement rather than aDestElement

https://reviewboard.mozilla.org/r/239174/#review244898
Attachment #8970403 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/be5ba8b6dea2
Make EditorBase::CloneAttributesWithTransaction() set sourceElement to aSourceElement rather than aDestElement r=m_kato
https://hg.mozilla.org/mozilla-central/rev/be5ba8b6dea2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.