After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 744141 - text vanishes when composer is detached after message arrives
text vanishes when composer is detached after message arrives
Status: RESOLVED FIXED
Product: geary
Classification: Other
Component: composer
master
Other Linux
: Normal normal
: ---
Assigned To: Geary Maintainers
Geary Maintainers
review
Depends on:
Blocks:
 
 
Reported: 2015-02-07 18:38 UTC by Adam Dingle
Modified: 2015-02-10 23:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Only call ComposerWidget.on_load_finished_and_realized() once (986 bytes, patch)
2015-02-08 06:27 UTC, Robert Schroll
accepted-commit_now Details | Review

Description Adam Dingle 2015-02-07 18:38:04 UTC
1. Press N to start composing a new message.  Do not detach it yet.

2. Type some text in the message body.

3. Use another email client to send yourself a message.

4. When the new message becomes visible in Geary, detach the message you are composing.

All message text will now vanish and there's no way to get it back.  I was burned by this this morning when I was writing a long email and detached the composer so I could read another message that had just arrived.
Comment 1 Robert Schroll 2015-02-07 20:10:50 UTC
I can confirm.  But it has nothing to do with the incoming email.  Simply detaching a new composer deletes its message contents.

This doesn't happen from an inline or paned composer.  Perhaps it was introduced when we moved the new composer from inline to paned, but I don't see how.  I'm very puzzled.
Comment 2 Robert Schroll 2015-02-08 06:27:33 UTC
Created attachment 296363 [details] [review]
Only call ComposerWidget.on_load_finished_and_realized() once

The actual problem was that any composer that first opened in the paned 
state would have its contents reset when it was detached.  There is code 
that can only run once the webview has loaded.  That used to reliably 
happen after the widget was realized, but when we got the draft resume 
code in, this was no longer the case.  There was some problem with it 
running before realization, so we put in a check, and attached to the 
realize signal if we weren't.  For reasons I don't understand, the paned 
composer also loads before realization, so we were following that code 
path.  When you detach a composer, it unrealizes and then realized 
again, and we hadn't disconnected that signal.  This code would run 
again, and one of things it did was set up the contents of the webview.

This patch disconnects from the signal, so this function only runs once.  
It does so unconditionally, but I haven't seen problems disconnecting a 
handler that was never connected.  Still, if we're worried, I can put in 
a guard.  I'm also not sure there's not a better way to set up something 
that will run only once two signals have occurred.
Comment 3 Jim Nelson 2015-02-10 22:12:53 UTC
Review of attachment 296363 [details] [review]:

That's fine, the guard is unnecessary; as you said, g_signal_disconnect() is safe that way.  However, please add a short comment explaining why it's being disconnected before committing.
Comment 4 Robert Schroll 2015-02-10 23:06:56 UTC
Attachment 296363 [details] pushed as c174ef3 - Only call ComposerWidget.on_load_finished_and_realized() once