Bug 48201 - REGRESSION Zimbra email compose is broken with the HTML5 parser
Summary: REGRESSION Zimbra email compose is broken with the HTML5 parser
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Evangelism (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Regression
Depends on:
Blocks: 41115
  Show dependency treegraph
 
Reported: 2010-10-23 21:43 PDT by Mihai Parparita
Modified: 2010-10-28 13:14 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mihai Parparita 2010-10-23 21:43:03 PDT
To reproduce:

1. Go to http://testzimbra.com/
2. Log in with username "zm1023yyxhm" and password "hvtpki"
3. Wait for all the UI to load
4. Click on the "New" button under the "Mail" tab to bring up a compose window

Expected result:
1. Compose window that you can type in

Actual result:
1. No editable area for the body of the email

This appears to be intermittent, so you may need to try it a few times to trigger it. Having an empty cache seems to make it more likely to happen. Using nightly builds I narrowed it down to the range r64727 to r64636. http://trac.webkit.org/changeset/64712, which enabled the HTML5 tree builder, was in that range. I verified that that particular change was responsible.

This is a beast of an app to reduce, but as far as I can tell, there's some sort of race condition with the body compose iframe. It has a src of http://demo2.zimbra.com/zimbra/public/blank.html. With the HTML5 parser change, I often see the compose area flash before it's replaced by the empty blank.html frame. Without the HTML5 change, "Failed to load resource: cancelled" message in the console about the blank.html frame. I'm guessing the app is registering an onload handler and enabling designMode on the frame and filling it with the new email template, and implicitly disabling the load of the blank.html source. That last part is no longer happening with the HTML5 parser, which is why blank.html is clobbering the compose area.

With things like bug 43423 I see that the HTML5 parser is affecting how events are fired for frames, but I don't know much else about this.
Comment 1 Adam Barth 2010-10-23 22:01:50 PDT
Any chance this is related to Bug 43328	?
Comment 2 Mihai Parparita 2010-10-23 22:08:02 PDT
It looks like this is the source of the compose mode editor, and specifically the iframe initialization:

http://google.com/codesearch/p?hl=en#l5SZa4ix2r4/Ajax/WebRoot/js/dwt/widgets/DwtHtmlEditor.js&q=DwtHtmlEditor&l=739

The code could definitely be race-y (there's a 50ms timeout after adding the iframe before trying to initialize it, etc.)
Comment 3 Mihai Parparita 2010-10-23 22:08:51 PDT
(In reply to comment #1)
> Any chance this is related to Bug 43328    ?

That regressed with the HTML5 lexer (r61234), this didn't regress till the HTML5 tree builder (r64712), so I don't think they're related.
Comment 4 Adam Barth 2010-10-23 22:10:51 PDT
Interesting.  Most of the timing related changes came in with the tokenizer.
Comment 5 Adam Barth 2010-10-26 00:42:24 PDT
The compose window is begin blown away when the first data come back from the network for blank.html.  Clearing the cache makes the bug more likely to occur because it delays the network load.
Comment 6 Adam Barth 2010-10-26 01:15:14 PDT
In Safari 5, I believe Zimbra is calling document.open or document.write on the iframe, which cancels the load.  However, in debugging TOT, I see that it's not calling those functions.  Instead, it's using innerHTML to set the content of the frame (which, of course, does not cancel the load).

The reason I believe it's calling implicitOpen in some form is that the contentDocument.URL of the iframe in Safari 5 is "http://testzimbra.com/".  The likely way for that to open is because the URL is inherited via implicitOpen.
Comment 7 Adam Barth 2010-10-26 01:28:52 PDT
Look like the code on the live site is slightly different than the code on codesearch:

DwtHtmlEditor.prototype._finishHtmlModeInit = function () {
    var a = this._getIframeDoc();
    try {
        if (AjxEnv.isSafari && a.body == null) {
            a.open();
            a.write("<html><head></head><body></body></html>");
            a.close()
        }
    }
    catch (t) {
        return
    }
    if (AjxEnv.isGeckoBased) {
        a.open();
        a.write(DwtHtmlEditor.INIT_HTML);
        a.close()
    }

    function e(i) {
        this._enableDesignMode(i);
        this._setContentOnTimer();
        this._updateState();
        this._htmlModeInited = true;
        this._registerEditorEventHandlers(document.getElementById(this._iFrameId), i)
    }
    if (AjxEnv.isIE) {
        setTimeout(AjxCallback.simpleClosure(e, this, a), DwtHtmlEditor._INITDELAY)
    } else {
        e.call(this, a)
    }
};
Comment 8 Adam Barth 2010-10-26 01:34:45 PDT
>         if (AjxEnv.isSafari && a.body == null) {

I suspect the difference is that in the old tree builder a.body was null at this program point but it's non-null now, which means the site no longer calls document.open() and it no longer cancels the load.

IMHO, this is an evangelism issue.  Life would be much better if they sent us down the Gecko codepath in this function.
Comment 9 Adam Barth 2010-10-28 13:14:00 PDT
The Zimbra folks have kindly fixed the issue on their end.