605481 - Firefox 4 doesn't render multiple TinyMCE instances correctly

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Spocke]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 ( .NET CLR 3.5.30729; .NET4.0E)
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:2.0b8pre) Gecko/20101019 Firefox/4.0b8pre

Firefox 4 doesn't render multiple editable iframes correctly in TinyMCE. The body becomes 10x10 pixels in size should be the width/height of a normal document body inside an iframe. This is a regression in behavior from Firefox 3 and it works correctly on all other browsers as well.

Reproducible: Always

Steps to Reproduce:
1. Open the attached URL.
2. Observe that only one of the editor instances gets contents rendered.
3. Try editing the ones that doesn't get contents. Notice that the body is 10x10 in size.
Actual Results:  
Body element gets rendered to small 10x10 pixels.

Expected Results:  
Should be auto in other words the width/height should be the size of the contents not a fixed value.

Comment 1

[(mostly gone) XtC4UaLL [:xtc4uall]]

Confirmed on XP with a Build built from http://hg.mozilla.org/mozilla-central/rev/cfd18201f49b.
Shouldn't this be in Editor?

Comment 2

[Mounir Lamouri (:mounir)]

I think this might be an editor bug too.
Ehsan, could you have a look?

Spocke, do you think you can found since when this is broken?

Comment 3

[Spocke]

I managed to pinpoint it to the nightly build of 2010-04-14 tested the Mac version, this is the first nightly that introduced the bug.

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2010/04/2010-04-14-03-mozilla-central/

Comment 4

[(no longer active)]

Here's the regression range:

<http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2010-04-13&enddate=2010-04-14>

Comment 5

[(no longer active)]

(In reply to comment #4)
> Here's the regression range:
> 
> <http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2010-04-13&enddate=2010-04-14>

Actually, the correct regression range is:

<http://hg.mozilla.org/mozilla-central/pushloghtml?tochange=92ba359340b1&fromchange=a02d1f25ce68>

Comment 6

[(no longer active)]

OK, I bisected this:

changeset:   40755:ac0109fc6043
user:        Olli Pettay <[e-mail]>
date:        Wed Apr 14 11:33:47 2010 +0300
summary:     Bug 557398, Show (i)frames asynchronously, r=jst sr=roc

Olli, can you take a look, please?

Comment 7

[Johnny Stenback (:jst)]

Blocking.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Olli, mind if I just steal this?  I agree that this looks like the same issue as bug 595752, and I think I have a decent handle on what's needed to fix this.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Feel free to take this. You know layout code a lot better than I do.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: and bug 595752. When showing a frameloader, give it the correct size up front, so we don't depend on the ordering of Show() and reflow of the subdocument frame.

Comment 11

[Boris Zbarsky [:bzbarsky]]

I wish I knew how to write a sane test for this... would need something that triggers a layout flush before the scriptrunner calling Show() fires; presumably this would need to be another scriptrunner that gets added during the same update batch.  So any test I write would be pretty fragile.  :(

Comment 12

[(no longer active)]

(In reply to comment #11)
> I wish I knew how to write a sane test for this... would need something that
> triggers a layout flush before the scriptrunner calling Show() fires;
> presumably this would need to be another scriptrunner that gets added during
> the same update batch.  So any test I write would be pretty fragile.  :(

Can't we have a reftest, with 605481-1.html containing two or three editable iframes, and 605481-1-ref.html containing the exact same iframes, only not editable?

Comment 13

[Boris Zbarsky [:bzbarsky]]

I have yet to figure out a testcase that reproduces the bug, period (other than the huge page in this bug and the throttled situation in bug 595752).  If you have a reasonably small one that shows the problem, I'd love to know!

Comment 14

[(no longer active)]

Hmm, I can't seem to reproduce it with normal designMode iframes...

Comment 15

[Boris Zbarsky [:bzbarsky]]

Right; that was my issue.

If someone can figure out what TinyMCE is doing to even put content in the iframes, that would be good...

Comment 16

[Spocke]

We basically generate a dynamic iframe and use document.open/write/close to insert a body structure, then load some CSS files and finally we set the contents of the body. It looks like some timing issue since when I tested on various builds to pinpoint the exact one that caused the problem different editors got rendered correctly. So making a simple test might be to fast?

Comment 17

[Boris Zbarsky [:bzbarsky]]

Spocke, this is most definitely a timing issue; see comment 11.

A far as comment 16 goes, what's the exact order?  createElement the iframe, appendChild the iframe, open/write/close?  Then change some styles on the iframe?  Then "set the contents of the body" whatever that means in this case?  Where in this sequence are things made editable and how?  I'd really like to make sure we don't break this again and all.  ;)

Comment 18

[(no longer active)]

There's also another option for testing this: shipping the TinyMCE version which reproduces the problem, but I'm not sure if that's the best approach.

Comment 19

[Boris Zbarsky [:bzbarsky]]

That might not be a bad idea, per se, if we also know what calls we need to make to trigger the problem...

Comment 20

[(no longer active)]

(In reply to comment #19)
> That might not be a bad idea, per se, if we also know what calls we need to
> make to trigger the problem...

Well, we could just copy the test case web page, couldn't we?

Comment 21

[Boris Zbarsky [:bzbarsky]]

Maybe.  With all its files (several JS and CSS files involved).  And we'd need to make sure to run it from http, etc....  Doable, but painful.  ;)

Comment 22

[(no longer active)]

(In reply to comment #21)
> Maybe.  With all its files (several JS and CSS files involved).  And we'd need
> to make sure to run it from http, etc....  Doable, but painful.  ;)

I think we can in-litmus? this for now if you agree.  I'm going to integrate a bunch of editing widgets into our test suite after 2.0, and TinyMCE could be one of them...

Comment 23

[Boris Zbarsky [:bzbarsky]]

Works for me!

Comment 24

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/4467f70ed6df

Comment 25

[Eelko Berkenpies]

(In reply to comment #24)
> Pushed http://hg.mozilla.org/mozilla-central/rev/4467f70ed6df

Whoohoo, thanks a lot! This commit seems to fix the (rather annoying) problem. :)

Running an up-to-date Minefield here using the latest TinyMCE (3.3.9.2). And I'm pretty confident it will work with older versions of TinyMCE too. :)

Comment 26

[Mike]

Hi - total Bugzilla noob, long time Firefox user here,

I'm using firefox-4.0b8pre.en-US.win64-x86_64 on Win 7 (updated this morning).

I'm still seeing a related issue when resizing the editor interface (height and/or width). When dragging the bottom right corner to adjust the size of the editor, the content contained (or added) in the editor does not adjust to fit the newly "saved" dimensions of the editor UI.

This can be reproduced on http://tinymce.moxiecode.com/examples/skins.php

When increasing the size of the editor, the content does not re-flow to fill the UI, but remains constrained to the previous UI dimensions.

When decreasing the size of the editor, the content flows outside the UI's new dimensions, overlapping other content on the page including other instances of the editor. You can see this on the test case by simply decreasing the size of the second editor.

After refreshing (ctrl+r or F5) the page, the content will re-flow to properly fill the newly "saved" UI dimensions.

As best I can tell this only triggers when there are multiple instances of the editor on the same page. It also seems there is usually one instance of the editor that is NOT affected. In the test case here, it's the first instance; on my WordPress installation (that also has 3 instances in place), the third instance functions properly.

I don't know if this is the same issue that was thought to be resolved or if a new bug should be started...

Comment 27

[Boris Zbarsky [:bzbarsky]]

New bug, please!