198506 - Text in XML pretty printing overlaps (first time only)

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect, P2)

Description

[piers]

The text in XML pretty printing seems to overlap. Collapsing then expanding the
element causes the text to be drawn correctly.

Test doc and screenshot to follow.

Comment 1

[piers]

Attachment: Sample XML document

Comment 2

[piers]

Attachment: Screenshot

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

this looks like a layout bug

Comment 4

[Nick Atkins]

Actually, on 2003033108 Win2k XML pretty print settings are ignored and XML is
no longer display - my XML page is rendered as plain text.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

that's bug 199484

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Reassigning to default owners. Oh, and this is a regression.

Comment 7

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Looks like some sort of font or text measurement problem.

Comment 9

[Steven Garrity [:sgarrity]]

refreshing the page also makes it flow properly - only happens on the initial
load (but it happens on the initial load consistently).

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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

Comment 11

[Axel Hecht]

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

Comment 12

[Olivier Cahagne]

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

Comment 13

[Christopher Hoess (gone)]

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

Comment 14

[Christopher Hoess (gone)]

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

Comment 15

[David A. Cobb]

Since Bug#210577 has been judged a dupe of this, the following symptoms that are
not covered here are also worth noting (quoted):

Steps to Reproduce:
1. Coldstart Mozilla Browser
2. Enter "http://www.w3.org/2001/04/infoset" in the Location input
3. EXAMINE THE DISPLAY -- possible to use ViewSource to check
4. Click on "Reload"
5. EXAMINE THE DISPLAY AGAIN

Actual Results:  
Got the heading panel notice about ". . . the document tree is shown"
The immediately obvious defects include --
The first block of comment ("this can be decoded . . ." )is not rendered at all,
only the "<!--" and "-->"
The third block, after <rdf:RDF>, ("This RDF schema's namespace name. . . .") is
just plain messy - bits of it are rendered.
Attribute names seem to run into the element names . . . (that's the
pretty-printing overlap described above).
And I can confirm the findings in comment#9: the failures are consistant on the
first rendering and go away if the page is refreshed.

Comment 16

[Jesper Hertel]

Same problem here (Mozilla 1.4 final on Windows 2000 - "Mozilla/5.0 (Windows; U;
Windows NT 5.0; en-US; rv:1.4) Gecko/20030624"). 

It shows the first time I view an XML document in a fresh Mozilla. The second
time I view an XML document, the display becomes correct. It doesn't matter
whether the second document is different from the first document, or identical
to the first document. It also doesn't matter what window it is in. Reload also
makes the display correct, as earlier mentioned -- this could also be seen as a
second display of an XML document.

Looks like an initialization problem?

(Maybe someone powerful (maybe the reporter) could change the Summary to "Text
in XML pretty printing overlaps on first display" or something like that? I
didn't find this bug when I first searched for it, because I searched for "first
xml". What is special about this bug is that it only appears on the *first* view.)

Comment 17

[csalsa]

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

Comment 18

[Michael Lefevre]

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

Comment 19

[Alex Vincent [:WeirdAl]]

FYI, I don't think reloading helps much.  I like to look at XBL via the
Monospace style sheet, and what happens for me is I get the overlap every time I
go to that style sheet.

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Hmmm.  A frame dump shows that the frames aren't overlapping, so it seems that
the text frames actually have the wrong size.

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So here's a rough idea of what's happening:
 * the XML prettyprinting code (triggered from nsXMLContentSink::DidBuildModel,
via MaybePrettyPrint) appends the entire transformed tree, which includes the
LINK elements that bring in stylesheets, as descendants of a DIV:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xml/document/src/nsXMLPrettyPrinter.cpp&rev=1.6&mark=191#180
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xml/document/resources/XMLPrettyPrint.xml&rev=1.6&mark=54-55#48

 * The insertion into the document kicks off the stylesheet loads for the two
stylesheets, which are asynchronous.

 * We do the initial reflow of the document (also triggered from
nsXMLContentSink::DidBuildModel, via StartLayout) with the stylesheets not
loaded yet

 * We go back to the main event loop, and the stylesheets are loaded, queueing
two style change reflows, one for a table frame and one for a block frame

 * These two style change reflows are merged using the reflow tree and done as a
single reflow.  In this reflow, the text frames aren't resized properly.  I'm
only seeing calls of nsTextFrame::Reflow in this reflow, and the reason is
eReflowReason_StyleChange (as it needs to be), but I think this needs to be
called more times.

I probably need to figure out which frames are getting the style changes queued...

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

That should have said "only seeing *2* calls of nsTextFrame::Reflow"

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The style change reflows are for the table and the div elements that are the
containers of the tree and the header, so that should cover things...

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The two text frames that are reflown are the text nodes for the text node "\n  
     This XML file does not appear to have any style information\n       
associated with it. The document tree is shown below.\n" (one text frame per
line, I presume).  So I think the style change reflow for the table is somehow
being suppressed.

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This fixes the bug.  It depends on the patch in bug 213333.

Comment 27

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Taking.

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm actually not sure if this is right, because the |switch
(aReflowState.reason)| in nsTableFrame::Reflow might already want to get
StyleChanged.  We do need to do "pass 1" as well for a style change.  But maybe
IR_StyleChanged should handle what needs to happen?

(This code seems far more complicated than it needs to be thanks to the
combination of our reflow reason/type system and the computation of min/max
widths using the same Reflow method as final size.)

Comment 29

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Some comments I wrote in email:

If the reflow reason is style change, the style change reason has to be
propagated to the descendants.  That's why I changed what I did.  Right
now tables are acting vastly differently when a style change reflow is
targeted at an ancestor or when it is targeted at the table itself.
That's wrong.  A table should do roughly the same thing when:
 (a) the 'font-size' property is changed on its parent, or
 (b) the 'font-size' property is changed on the table itself.
Style change reflows should only be used for style changes, so
propagating the reflow reason correctly shouldn't even affect that many
cases.
                                                                                
Furthermore, a style change reflow *cannot* be optimized away, since
style change reflows are coalesced -- if a style change affects the
style of an element and some of its descendants, we'll do a style change
reflow targeted only at the ancestor.  This happens if it's an inherited
property that's changing and that's inherited, or even if multiple
properties are changing.
                                                                                
In other words, "style change" is the reflow type that says "wipe out
everything and start over".  Resize reflow allow preservation of
intrinsic width information and other optimizations.  All the other
reflow types (I think -- I'm saying this from memory, and I might have
forgotten one, but I hope not) mean whatever the caller in that
particular case wants them to mean.

Comment 30

[Bernd]

I thought that I fixed all those style change -> resize reason changes in bug
108340. Obviously I missed one file :-(. With some more thought I think
nsTableFrame::IR_StyleChanged should be similiar to 
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableRowFrame.cpp#1216
. Then the the reflow will be passed as a style change reflow . As this will
probably influence the table layout setting the flag to initialize the strategy
again will still be necessary. To bring the table in accordance with the new
layout a resize (nextReason = eReflowReason_Resize;) with all optimizations
would do the job.

This would also make the cases when the incremental reflow is targeted at the
table or its childs more symmetrical.

Comment 31

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2025-10-27)]

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

Comment 32

[Bernd]

Attachment: patch

I believe it would be better to fix this inside the style change routine

Comment 33

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 34

[Erik]

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

Comment 35

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: testcase

This testcase shows that neither patch is correct.

Comment 36

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Since neither patch actually does the right thing for resizing the tables, I
think I prefer my patch, since it at least doesn't cause more reflows than we
had before, whereas I think Bernd's patch does.

Comment 37

[Bernd]

I dont see where my patch fails with the testcase (win98).

Comment 38

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

After pressing the "Resize TABLEs" button, the first table isn't resized, so its
text ends up on two lines, and the third and fourth tables aren't resized, so
that they don't contain all their text.

Comment 39

[Bernd]

David are you sure that you applied my patch? I see that the current trunk and
your patch fail on the testcase, but with my patch it looks OK.
for  the first table
trunk:      table does not resize, text does exceed the table cell
your patch: table does not resize horizontal, text is wrapped
my patch:   table does resize horizontal, text does not wrap, text is fully
contained in the table cell.

If you are sure that you applied the patch, could you attach a reflow log for
the first table.

Comment 40

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 128884 [details] [diff] [review]
patch

I must have tested the wrong tree.

Comment 41

[Wira Mulia]

Try to activate autoscroll with middle click (Firebird) on the XML page, it will
mess the format even further.

Comment 42

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

that's bug 215762

Comment 43

[Brendan Eich [:brendan]]

Comment on attachment 128884 [details] [diff] [review]
patch

approved for 1.5b.

/be

Comment 44

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

->Bernd

Comment 45

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk (at Bernd's request), 2003-08-13 23:15 -0700.

Comment 46

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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

Comment 47

[Heikki Toivonen (remove -bugzilla when emailing directly)]

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

Comment 48

[Oliver Klee]

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

Comment 49

[Bernd]

If this gets approved, David could you please check it in, I don't build
branches. Thanks

Comment 50

[Asa Dotzler [:asa]]

Comment on attachment 128884 [details] [diff] [review]
patch

This is not going to make 1.4.1.  Please re-request aproval after 1.4.1 ships
if you'd like to get this in for 1.4.2.

Comment 51

[Asa Dotzler [:asa]]

Comment on attachment 128884 [details] [diff] [review]
patch

approved for 1.4.1

Comment 52

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to MOZILLA_1_4_BRANCH, 2003-09-04 06:55 -0700.

Comment 53

[Boris Zbarsky [:bzbarsky]]

Note that this caused bug 222467 

Comment 54

[Olivier Cahagne]

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

Comment 55

[Asa Dotzler [:asa]]

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

Comment 56

[Biju]

something similar happens here too
http://www.meyerweb.com/eric/css/edge/raggedfloat/demo.html

Is bug 237139 a duplicate