Closed
Bug 85554
Opened 23 years ago
Closed 23 years ago
<?xml-stylesheet?> that's outside of prolog is applied
Categories
(Core :: XML, defect, P3)
Core
XML
Tracking
()
VERIFIED
FIXED
mozilla0.9.4
People
(Reporter: chrispetersen, Assigned: hjtoi-bugzilla)
References
()
Details
Attachments
(4 files)
1.30 KB,
application/octet-stream
|
Details | |
1.72 KB,
patch
|
Details | Diff | Splinter Review | |
2.62 KB,
patch
|
Details | Diff | Splinter Review | |
6.11 KB,
application/octet-stream
|
Details |
Build: 2001060713 Platform: All Expected Results: Stylesheet PI should be ignored if outside of the document's prolog. What I got: Stylesheet is applied to the document Steps to reproduce: 1) Go to any of these tests: http://mozilla.org/quality/browser/standards/xml/pi_in_root_element.xml http://mozilla.org/quality/browser/standards/xml/pi_in_child_element.xml http://mozilla.org/quality/browser/standards/xml/pi_after_root_element.xml 2) The stylesheet PI is not located in the document's prolog but it style applied to the text.
Assignee | ||
Comment 2•23 years ago
|
||
Peter, I thought you might be interested to learn about this bug. We are a bit too forgiving with our handling of the xml-stylesheet PI. This is kinda unfortunate because people are already learning to depend on this incorrect behavior. This is probably easy to fix. In the sink we have the prolog state, but that does not solve the whole problem. If one adds the PI with script after the document is loaded one needs to check that the PI has no parent elements and no elements before it.
Keywords: correctness
Comment 3•23 years ago
|
||
Heikki, I agree. I was very surprised that no check was done either for <link> elements (to see if they're within head). However, I didn't want to change the code as part of my patch for bug 7515, which turned out to cause enough regressions as is :(. We do need the check in the content sink so it applies to XSLT stylesheets too. And we could add a check to nsStyleLinkElement::UpdateStylesheet, calling a new function in nsIStyleSheetLinkingElement that checks if the node is in the right "spot". We then need to implement that function in the three stylesheet linking elements: HTMLLink, HTMLStyle and XMLPI.
Assignee | ||
Updated•23 years ago
|
Priority: -- → P3
Target Milestone: --- → mozilla0.9.3
Comment 5•23 years ago
|
||
Doesn't look like this is getting fixed before the freeze tomorrow night. Pushing out a milestone. Please correct if I'm mistaken.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Assignee | ||
Comment 6•23 years ago
|
||
Assignee | ||
Comment 7•23 years ago
|
||
Assignee | ||
Comment 8•23 years ago
|
||
The zip of testfiles contains 5 XML files and 2 CSS files. The first three tests have the PI somewhere in the prolog, which should cause the document text to be green. p4-p5 have the PI in the document and in the epilog, and the text color should NOT be red. Note that IE 5.x fails test p2 where the PI is in the DOCTYPE, but it passes all of the other tests. The fix went into the PI element code. I have not yet tested with XSLT to see if this is enough. I might need to add a little check in the XML content sink or something.
Status: NEW → ASSIGNED
Whiteboard: [fixinhand]
Assignee | ||
Comment 9•23 years ago
|
||
Assignee | ||
Comment 10•23 years ago
|
||
Turns out I need to check in the XML content sink if we are in the prolog. I will still check with scripts adding stuff, but I think/hope this is enough (I'll attach more tests). Peter, can you review?
Comment 11•23 years ago
|
||
r=peterv.
Assignee | ||
Comment 12•23 years ago
|
||
Assignee | ||
Comment 13•23 years ago
|
||
With some more testing it seems things work perfectly with CSS. Things also work in XSLT expect when the PI is added by script (in which case we don't seem to do the XSLT transformation and I did not change that behaviour). Peter, is there a bug on that?
Comment 14•23 years ago
|
||
Looks ok to me, sr=jst. One style nit: - if (target.EqualsWithConversion(kStyleSheetPI) && !type.EqualsIgnoreCase("text/css")) { + if (mState == eXMLContentSinkState_InProlog && + target.EqualsWithConversion(kStyleSheetPI) && + !type.EqualsIgnoreCase("text/css")) { shouldn't the two last lines here be indented to line up with mState?
Assignee | ||
Comment 15•23 years ago
|
||
Fixed, time for wine.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: [fixinhand]
Reporter | ||
Comment 16•23 years ago
|
||
Marking verified in the Sept 05 th build.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•