Closed Bug 604493 Opened 9 years ago Closed 9 years ago

Composition Window broken on trunk builds due to tracemonkey landing (merge of compartments) (cannot compose, write, reply to messages)

Categories

(MailNews Core :: Composition, defect, blocker)

defect
Not set
blocker

Tracking

(blocking-seamonkey2.1 b2+)

VERIFIED FIXED
Tracking Status
blocking-seamonkey2.1 --- b2+

People

(Reporter: standard8, Assigned: neil)

References

Details

(Keywords: regression)

Attachments

(1 file)

Thunderbird's compose window is broken on trunk following the tracemonkey merge of compartments.

Symptoms:

- Only one email address box is displayed
- Can't focus the body section of the compose window to be able to type email.

It looks like at some stage the load of the compose window and its document is stopped/doesn't finish.

Unfortunately I won't be able to offer more debug until tomorrow, but filing this as the tracker so that others can help as well.
Flags: blocking-thunderbird-next+
The mozmill part was fixed in bug 604363.
No, that was different -- Mozmill refused to work entirely. These failures appeared after the patch in bug 604363 landed.
Duplicate of this bug: 604586
Duplicate of this bug: 604579
Both SeaMonkey Editor and Message Compose are broken.
Something's going wrong in makeEditable:

this.editingSession.makeWindowEditable(this.contentWindow, editortype,  waitForUrlLoad, true, false);

The editor element's contentWindow property:

this.boxObject
    .QueryInterface(Components.interfaces.nsIContainerBoxObject)
    .docShell
    .QueryInterface(Components.interfaces.nsIInterfaceRequestor)
    .getInterface(Components.interfaces.nsIDOMWindow);

appears to be returning the outer window. However by the time it reaches nsEditingSession::MakeWindowEditable it's an inner window.
I'm going to blame compartments, in particular, Xray Wrappers. Over to Blake?
blocking2.0: --- → ?
I think part of the editor may be broken.

If I try loading the midas demo (http://www.mozilla.org/editor/midasdemo/) in a content tab in Thunderbird, then it loads and works, however in the error console I see:

WARNING: NS_ENSURE_TRUE(mDoneSetup) failed: file /Users/moztest/comm/main/src/mozilla/editor/composer/src/nsEditingSession.cpp, line 561
WARNING: NS_ENSURE_TRUE(hasTransaction) failed: file /Users/moztest/comm/main/src/mozilla/editor/libeditor/base/nsEditor.cpp, line 755
WARNING: NS_ENSURE_TRUE(inOffset) failed: file /Users/moztest/comm/main/src/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 4634
WARNING: NS_ENSURE_TRUE(temp) failed: file /Users/moztest/comm/main/src/mozilla/editor/libeditor/html/nsHTMLEditor.cpp, line 4611

Doing some analysis on the compose window startup, I see two things, the first a similar warning:

WARNING: NS_ENSURE_TRUE(mDoneSetup) failed: file /Users/moztest/comm/main/src/mozilla/editor/composer/src/nsEditingSession.cpp, line 561

Secondly, we try to load a document (about:blank) into the editor and this load never completes - this indicates we're missing the "obs_documentCreated" notification that I believe comes from the editor.
FWIW, this bug was not yet present in (among others I'm sure) the SeaMonkey linux-i686 nightly dated 2010-10-13; however it was present in build ID 20101014004215 (see bug 604579 comment #0) which places it a little early to be due to the fix for bug 604363 ( http://hg.mozilla.org/mozilla-central/rev/ed24c21e6497 dated Thu Oct 14 14:56:31 2010 -0700). But maybe I misunderstand comment #3 above.
(In reply to comment #10)
> FWIW, this bug was not yet present in (among others I'm sure) the SeaMonkey
> linux-i686 nightly dated 2010-10-13; however it was present in build ID
> 20101014004215 (see bug 604579 comment #0) which places it a little early to be
> due to the fix for bug 604363 (
> http://hg.mozilla.org/mozilla-central/rev/ed24c21e6497 dated Thu Oct 14
> 14:56:31 2010 -0700). But maybe I misunderstand comment #3 above.

Yeah, bug 604363 is unrelated.
(In reply to comment #9)
> I think part of the editor may be broken.
> 
> If I try loading the midas demo (http://www.mozilla.org/editor/midasdemo/) in a
> content tab in Thunderbird, then it loads and works, however in the error
> console I see:

I've just double-checked, the editor stuff was broken before the compartments landing, so the warnings there aren't anything to do with this specific bug.
Duplicate of this bug: 604606
Duplicate of this bug: 604619
I've made a debug build and compared the log entries while opening the compose window with a log from a debug build I've made last week. The logs are very identical, I see only two new entries, but I have no idea if this is related:

WARNING: NS_ENSURE_TRUE(editor) failed: file /Volumes/Developer/temp/src/mozilla/editor/libeditor/base/nsEditorCommands.cpp, line 549

###!!! ASSERTION: no editor: 'htmlEditor', file /Volumes/Developer/temp/src/mailnews/compose/src/nsMsgCompose.cpp, line 1372


Hope this helps.
(In reply to comment #10)

> FWIW, this bug was not yet present in (among others I'm sure) the SeaMonkey
> linux-i686 nightly dated 2010-10-13; however it was present in build ID
> 20101014004215

My bug window on SM-Trunk is: 'last good' 20101013012529
                              'first bad' 20101013210207
(In reply to comment #9)
> Secondly, we try to load a document (about:blank) into the editor and this load
> never completes - this indicates we're missing the "obs_documentCreated"
> notification that I believe comes from the editor.
This is fallout from the inner window being passed into makeEditable - when the load completes it fires against the outer window so we ignore it.
Attached patch Possible patchSplinter Review
Blake says that JS now likes to provide inner windows. So rather than fiddle about with whether a window is inner or outer, I just switched the code to comparing docshells instead.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #483598 - Flags: review?(mrbkap)
(In reply to comment #18)
>
> Blake says that JS now likes to provide inner windows. So rather than fiddle
> about with whether a window is inner or outer, I just switched the code to
> comparing docshells instead.

https://bugzilla.mozilla.org/show_bug.cgi?id=603533#c6
Seems to back that up, but doesn't say when/how that happened.
Maybe a clue in bug 603533
Status: ASSIGNED → NEW
(In reply to comment #18)
> Created attachment 483598 [details] [diff] [review]
> Possible patch
> 
> Blake says that JS now likes to provide inner windows. So rather than fiddle
> about with whether a window is inner or outer, I just switched the code to
> comparing docshells instead.

I've tried this patch through the try server, and whilst it appears to fix some things, the content policy is still broken:

TEST-UNEXPECTED-FAIL | test-compose-mailto.js | test_checkInsertImage
  EXCEPTION: Loading of image has been unexpectedly blocked in a mailto compose window
    at: test-compose-mailto.js line 123
       test_checkInsertImage() test-compose-mailto.js 123

TEST-UNEXPECTED-FAIL | test-general-content-policy.js | test_generalContentPolicy
  EXCEPTION: Image has not been allowed in reply window as expected.
    at: test-general-content-policy.js line 212
       checkComposeWindow([object Object],true,true) test-general-content-policy.js 212
       test_generalContentPolicy() test-general-content-policy.js 298

The exception says pretty much what is happening.

The relevant function does reference nsIDOMWindowInternal so we may need to update that as well:

http://hg.mozilla.org/comm-central/annotate/d7fd18b29794/mailnews/base/src/nsMsgContentPolicy.cpp#l588
OK, so the compose service tracks compose windows via their DOM window, which is passed in form MsgComposeCommands.js, and is now an inner window, but the content policy gets given an outer window, thus it fails to find a match...
Keywords: useless-UI
(In reply to comment #20)
> (In reply to comment #18)

> I've tried this patch through the try server, and whilst it appears to fix some
> things, the content policy is still broken:
> 
> TEST-UNEXPECTED-FAIL | test-compose-mailto.js | test_checkInsertImage
>   EXCEPTION: Loading of image has been unexpectedly blocked in a mailto compose
> window
>     at: test-compose-mailto.js line 123
>        test_checkInsertImage() test-compose-mailto.js 123
> 
> TEST-UNEXPECTED-FAIL | test-general-content-policy.js |
> test_generalContentPolicy
>   EXCEPTION: Image has not been allowed in reply window as expected.
>     at: test-general-content-policy.js line 212
>        checkComposeWindow([object Object],true,true)
> test-general-content-policy.js 212
>        test_generalContentPolicy() test-general-content-policy.js 298
> 
> The exception says pretty much what is happening.
> 
If I've gotten the right tryserver build (Built from http://hg.mozilla.org/mozilla-central/rev/397c458b40a4), the above must be test failures only, since they work fine for me in realtime testing.
(In reply to comment #22)
> (In reply to comment #20)
> > (In reply to comment #18)
> > I've tried this patch through the try server, and whilst it appears to fix
> > some things, the content policy is still broken:
> If I've gotten the right tryserver build (Built from
> http://hg.mozilla.org/mozilla-central/rev/397c458b40a4), the above must be test
> failures only, since they work fine for me in realtime testing.
Did you try inserting a local image into your message?
(In reply to comment #23)

> Did you try inserting a local image into your message?

Yes,
while it just shows the placeholder in the composition window, the image is actually inserted when sent/saved.
I could deal with that in the interim.
> while it just shows the placeholder in the composition window, the image is
> actually inserted when sent/saved.
> I could deal with that in the interim.
It's still somewhat broken UI from a UX viewpoint.
(In reply to comment #24)
> (In reply to comment #23)
> 
> > Did you try inserting a local image into your message?
> 
> Yes,
> while it just shows the placeholder in the composition window, the image is
> actually inserted when sent/saved.
> I could deal with that in the interim.

That's a failure then, not a pass. The test is testing whether or not the remote content (in this case an image) is *loaded/shown* not if it gets sent.

If I understand Neil's response to mine correctly, then we just need to update the content policy to account for the change - that can be done separately to the current patch when we get time. Hence I think the only thing blocking the current patch is getting the review and approval.
Did this issue also break Chatzilla?

Opening cZ now opens to a blank window.  The *client* window is blank.

Nonetheless, you can still /join seamonkey from there (or open irc://irc.mozilla.org/seamonkey directly), & that window will open.  The list of users will display.  You can even type comments, they are accepted (viewable in a different client), but the comment window is & remains totally blank (nothing is echoed back).

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a0cca0e817e8&tochange=047cb442023a

(Windows on my end.)
Chatzilla is nothing to do with the compose window, please file a separate bug.
Duplicate of this bug: 605007
Summary: Composition Window broken on trunk builds due to tracemonkey landing (merge of compartments) → Composition Window broken on trunk builds due to tracemonkey landing (merge of compartments) (cannot compose, write, reply to messages)
Blocks: 605140
Attachment #483598 - Flags: review?(mrbkap) → review+
Proposed patch works perfectly for me. Great to see working composition windows ;)
blocking-seamonkey2.1: --- → b2+
Comment on attachment 483598 [details] [diff] [review]
Possible patch

Seeking approval for this compartments regression.
Attachment #483598 - Flags: approval2.0?
Comment on attachment 483598 [details] [diff] [review]
Possible patch

a=beltzner
Attachment #483598 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/8012dab3b01c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101020 Firefox/4.0b8pre SeaMonkey/2.1b2pre - Build ID: 20101020033222

I VERIFY that the bug is not present in this nightly when writing an email in plaintext. I don't use the HTML editor, and IIUC, according to doctrine the fix should be checked on other apps (Shredder) and platforms (Windows, Mac) before setting the Resolution to VERIFIED.
Status: RESOLVED → VERIFIED
blocking2.0: ? → ---
I have a problem with reply becoming empty when the message was send to my gmail.com address with the "+" ability from gmail. Gmail gives you the possibility to have multiple addresses by inserting a "+":
Example:
name@gmail.com (which works fine no problem with reply!)
Now for instance I'd like to use name+sale@gmail.com (it is a gmail feature!)
It will be sent to name@gmail.com and can make a filter in the "+sale".
But when I want to reply the original text is cleared!
And now comes the tricky part:
When I reply on a message I sent the problem does not occur.
Only when I reply to someone else! So someone sent me a mail using name+sale@gmail.com and when I try to reply the original text is cleared.
Bart, this bug is closed and verified fixed, and your problem doesn't seem to
be related to this issue. After exhausting any support available from either http://getsatisfaction.com/mozilla_messaging/topics or (user-to-user forum) http://forums.mozillazine.org/viewforum.php?f=39 please file a new bug if the issue cannot be resolved and if it hasn't been filed yet in another bug.
You need to log in before you can comment on or make changes to this bug.