186835 - {ib}CSS style sheet not working if XML is translated via XSL to XML-with-a-CSS-style-sheet

Status: VERIFIED FIXED

(Core :: XSLT, defect)

Description

[Brian Downing]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20021130
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3b) Gecko/20021223

XML translated to XML with a CSS stylesheet processing tag does not appear to
run the CSS stylesheet correctly.  No visible output is generated if there is a
CSS stylesheet that styles the tags output by the XSL stylesheet.  If the CSS
file is missing or empty, you get unstyled output.  If the XSL processing is
done ahead of time and the postprocessed XML loaded into Mozilla, the CSS
styling works as expected.

Both files appear to have identical DOM trees.

I do not believe this is a dup of the stylesheet caching bugs (155715, etc.) as
a browser restart does not fix anything, and changes to the files as described
above appear on reload.

I'm classifying it as Major only because I don't know of another way to both
translate and style XML without resorting to XHTML.

Reproducible: Always

Steps to Reproduce:
1. Prepare directory with test.xml, test-processed.xml, test.xsl, and test.css
2. Load test.xml
3. Load test-processed.xml (which is test.xml with the XSLT processing already done)
4. Compare

Actual Results:  
test.xml is not rendered.  test-processed.xml is rendered fine.  Their resultant
DOM trees in the DOM inspector appear to be identical - they both have the
proper XML tags and stylesheets.

Expected Results:  
I think the results should be identical.

Comment 1

[Brian Downing]

Attachment: XML test case before XSL transformation

Comment 2

[Brian Downing]

Attachment: XML test case after XSL translation

Comment 3

[Brian Downing]

Attachment: XSL stylesheet for test case

Comment 4

[Brian Downing]

Attachment: CSS stylesheet for test case

Comment 5

[Boris Zbarsky [:bzbarsky]]

Is this only a problem with current trunk builds, or also with 1.2.1?

Comment 6

[Brian Downing]

1.2.1 is affected as well.

Comment 7

[Boris Zbarsky [:bzbarsky]]

OK.  Out of curiousity, does opening the "File" menu do anything to change the 
rendering in the "non-rendering" case?

Comment 8

[Brian Downing]

Just opening the file menu?  No, not in 1.2.1 or the trunk build from Dec. 23.

Comment 9

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

->XSLT

Comment 10

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

Strange, the stylesheet gets hooked up, but the rules in it cause everything to
get hidden. If I change the rule:

block {
  display: block;
}

to

block {
  display: inline;
}

everything works as expected.

Comment 11

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

seems to be a block-in-inline problem. If I add a rule to make the root node a
block as well everything works as excepted.

so the following doesn't work if generated through XSLT:

inline          <- A
  block         <- B
    inline
      #text
    inline
      #text

but works fine if loaded directly. Whereas if the A node is changed to a block,
or if the B node is changed to a inline everything works even when loaded
through XSLT.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Hmm.. The only way an inline root node can work is if all the content gets 
frames constructed at once.  If the root node's frame is constructed and then 
we get some ContentInserted/ContentAppended calls, we will flub the frame 
construction.

I bet that's the difference between just loading the document (constructs the 
DOM and then kicks off frame construction) and building it via XSLT (builds DOM 
incrementally with notifications along the way??)

Comment 13

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

there should be no ContentInserted/ContentAppended calls after layout is
started. However the linked stylesheet won't be loaded until after the initial
reflow so that's probably what causes this to fail. 

Comment 14

[Boris Zbarsky [:bzbarsky]]

Same thing; a reframe due to a display change is implemented via a pair of 
calls to 
nsCSSFrameConstructor::ContentRemoved/nsCSSFrameConstructor::ContentInserted.

Comment 15

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

Attachment: block layout until all stylesheets are loaded

This holds off the notification that ends up starting layout until all linked
stylesheets in the result document are loaded. This should take care of all
cases where the stylesystem or layout doesn't fully deal with the incremental
changes of stylesheets comming in after the initial reflow.

The patch uses the existing mechanism in the nsCSSLoader that notifies an
observer when a stylesheet is fully loaded (including child-sheets). To do this
i've added a parameter to nsIStyleSheetLinkingElement::UpdateStylesheet that
lets you specify an observer to be notified when the stylesheet for the element
is done loading.


As a sidenote, the contentsinks should be using this same mechanism to unblock
the parser. I've filed bug 186943 on this.

Comment 16

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

I removed the comment in txMozillaXMLOutput::startHTMLElement about "XXX Trick
nsCSSLoader into blocking/notifying us?" since this patch does just that.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 110250 [details] [diff] [review]
block layout until all stylesheets are loaded

> +    // aSheet might not be in our list if the load was done syncronously

"synchronously"

sr=me otherwise, though we should remove all this code once sheets no longer
block the parser...

That said, this is all wallpaper over the {ib} issues which we need to fix for
other reasons and I'm not completely convinced we want to spend time doing this
instead of addressing that core issue...

Comment 18

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

bz: Agreed. The main reason i want to do this is that we will otherwise keep
getting bugs on XSLT where "it works for normal pages but not if I use XSLT".

Comment 19

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

->me

Comment 20

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

Checked in fix for XSLT.

Boris: Are there a bug on the underlying inline-in-block problem, or do you want
to use this bug for that?

Comment 21

[Boris Zbarsky [:bzbarsky]]

Um... see the "depends on" field of this bug, eh?  ;)  That will give you (via
its dependencies) all the {ib} goodness you want.

Comment 22

[Aleksey Nogin]

This commit have added a "uninitialized" warning on Brad TBox:

+extensions/transformiix/source/xslt/txMozillaXMLOutput.cpp:496
+ `nsresult rv' might be used uninitialized in this function

Indeed, it appears that the (rv == NS_ERROR_HTMLPARSER_BLOCK) comparison is
quite likely to be comparing an uninitialized rv. Is one of the previous lines
missing an "rv = "?

P.S. Per bug 179819 such warnings ought to be considered compilation errors.

Comment 23

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

Attachment: eek!

thanks for catching this

Comment 24

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

Comment on attachment 110811 [details] [diff] [review]
eek!

got r=glazman over irc

Comment 25

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 110811 [details] [diff] [review]
eek!

r=glazman !!!

Comment 26

[Johnny Stenback (:jst)]

Comment on attachment 110811 [details] [diff] [review]
eek!

sr=jst

Comment 27

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

warning-fixes checked in

Comment 28

[Axel Hecht]

mass verifying

Comment 29

[Ruvim Pinka]

Attachment: testcase xml+xslt online

Just to check the original testcase in browser online (without downloading).