Loading an XML file that is large enough that scrollbars are needed to show it will cause the XML file to not show up at all until the window size is changed to trigger a reflow. Dbaron suggested backing out his fix for bug 156522 and that fixes the problem.

Load this in a window small enough that it needs scrollbars and this won't show, load in a window large enough that you don't need the scrollbars and it'll work just fine.

This is fixed by heikki's patch in nsXMLDocument.cpp, which was checked in only to the branch but not the trunk: ---------------------------- revision 1.149.2.4 date: 2002/05/18 00:03:39; author: heikki%netscape.com; state: Exp; lines: +1 -0 branches: 1.149.2.4.2; Bug 81546, workaround to make XHTML documents with forms to display. r=harishd, sr=jst, a=ADT,chofmann,brendan ---------------------------- Should we consider checking this patch into the trunk pending further investigation?

The patch in question is attachment 71687 [details] [diff] [review].

I'd argue that this is not "fixed" by Heikki's change, the problem might be worked around by that change, but there's still a problem...

*** Bug 157409 has been marked as a duplicate of this bug. ***

Go to: http://212.181.21.47/xml/records.xml and you will be able to reproduce this very easily. The content is shown after a resize. And reloading the page makes the scrollbars not to work, only resizing again makes them work.

A few more details about why this happens: In the old world (before bug 156522), the AttributeChanged on the scrollbar would trigger XUL layout logic that would handle the damage without posting a new reflow command. (I'm not quite sure how it does that, but box has its mysterious ways sometimes. Perhaps it was synchronous.) In any case, after bug 156522, moving the slider causes a reflow command to be posted during the initial reflow, which is not the typical case, although I suspect it does seem to happen in other cases (witness bug 81546). In other words, my change means that, if a scrolllbar is present, we're now posting a reflow command in the middle of the initial reflow, where we weren't doing so before. If there's a reflow command posted, then PresShell::UnsuppressPainting won't actually unsupress -- instead it will set mShouldUnsuppressPainting, which indicates that we *should* unsuppress after the next call to ProcessReflowCommands. However, ProcessReflowCommands is never called to process any reflows that were queued during the initial reflow.

*** Bug 157611 has been marked as a duplicate of this bug. ***

So it sounds like the real problem is that reflow commands that were queued up during the initial reflow are never processed.... ProcessReflowCommands is called from the following places: 1) FlushPendingNotifications() 2) ReflowEvent::HandleEvent(), but only if the shell is not batching reflows (that is, always except in editor, pretty much). That's it. It seems that the "right fix" is that at the end of InitialReflow we want to post a reflow event if the reflow queue is not empty.

Fixes this bug and bug 81546 (tested on Johnny's testcase, on the url in bug 157611, using about:blank as default homepage, URL in comment 7, the testcases in bug 81546). This code is basically the code that ProcessReflowCommands executes if it gets interrupted while the reflow command queue is not empty. Thoughts?

Also, see bug 81546 comment 63. The reason this is not a problem for HTML is that InitialReflow for HTML is called before all the content is in place. So more reflows happen in HTML documents as content is inserted into the content model and reflow commands are placed in the queue (AppendReflowCommand posts a reflow event).

That's the left-over that PresShell::DidCauseReflow() is supposed to cleanup in its call to |FlushPendingNotifications| before returning, but the |if| there is preventing it from doing it sometimes. Seems that it is DidCauseReflow() that might need some care.

Yup. You're right. So it seems to me that the correct behavior here is to _always_ post a reflow event in DidCauseReflow (unless we're doing a synchronous flush due to gAsyncReflowDuringDocLoad being false). PostReflowEvent() already hadles things like an event already being posted, the queue being empty, etc, so we don't need to worry about that. I can think of no reason to restrict the posting to the cases when mDocumentLoading is true, though I cannot offhand think of a way to set up a situation in which one of the other callers of DidCauseReflow() will leave stuff in the queue without posting a reflow event....

Comment on attachment 91690 [details] [diff] [review] Bang on DidCauseReflow instead r=dbaron, although I admit not understanding the logic around gAsyncReflowDuringDocLoad, etc.

Comment on attachment 91690 [details] [diff] [review] Bang on DidCauseReflow instead sr=rbs (On a different note, now that <iframe> & friends are being handled in the content, I wonder if the dummy request is of any use. Might be worth investigating at some stage if it can be removed since it may be adding undue complications to the code now.)

The dummy request is still needed for plugins, and maybe for images too.

Oh, and for <frame> elements too, they still load their data from the frame code...

Does this patch fix bug 157487?

Yes.

Does this happen on the branch too? If so, we should get this in there and remove the hack in nsXMLDocument::EndLoad(). PS. Thanks for getting this sucker fixed!

May as well take this. ;)

Comment on attachment 91690 [details] [diff] [review] Bang on DidCauseReflow instead a=scc for checkin to the mozilla trunk

Checked in on the trunk.

amar: when verifying this as fixed, pls also check to see if boris's checkin resolved bug 157487. if it does, pls mark bug 157487 as wfm, with a comment that this bug 156985 resolved it. then, pls add a comment to bug 156522, that bug 157487 no longer blocks bug 156522.

a=chofmann for 1.0.1 add fixed1.0.1 keyword after checking into the branch

chofmann: from my conversation with Kevin, i understood that the patch for bug 156522 will include the needed attachments from the various dependent bugs. do we need this patch applied to the branch? kevin: if we need this one on the branch too (to support bug 156522), pls nominate it by adding adt1.0.1 to the status whiteboard. thanks!

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending drivers' approval. pls check this in to the 1.0 branch, then replace "mozilla1.0.1+" with "fixed1.0.1". thanks!

checked in attachment 91690 [details] [diff] [review] to the 1.0 branch.

*** Bug 158541 has been marked as a duplicate of this bug. ***

Works fine with todays branch and trunk builds: 2002-07-22-08-1.0 and 2002-07-08-trunk. The given XML testcase shows up first time when I load it in a small window.

Replacing "fixed1.0.1" with "verfied1.0.1" per Comment #31 From Amarendra Hanumanula.