Closed Bug 135895 Opened 23 years ago Closed 23 years ago

XHTML <style> element not working


(Core :: CSS Parsing and Computation, defect, P1)






(Reporter: john, Assigned: peterv)




(Keywords: regression, Whiteboard: [fixed on trunk][Hixie-P1][Hixie-1.0][ADT1] affects W3C Selectors test suite)


(3 files, 3 obsolete files)

In the build as of the 3rd, attachment 75083 [details] was showing up with lots of style.
 Today the styles specified in the page seem to not show up at all.  The only
thing "out of the ordinary" on this page is that it is XHTML Strict.
Oh, some people believe this is a regression of bug 34849, which seems likely.
Yes. The XML content sink is blocking the load of the stylesheet for STYLE
elements with inline style (it used to handle those on its own). I've removed
the blocking (InitStyleLinkElement(nsnull, PR_TRUE)) and moved the updating of
the stylesheet from StartElement to EndElement, when we know that the STYLE
element has all of its children (including the inline style's textnode). I've
also made the HTMLStyleElement always mark itself as inline style if it's in an
XML document, we used to prevent <style src="..."> for XHTML in the XML content
sink but now that the element does all the style loading on its own, it has to
be handled there. Looking for r and sr on the patch I'm about to attach.
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.0
Attached patch v1 (obsolete) — Splinter Review
Attached patch v1 (obsolete) — Splinter Review
Eurk, wrong patch.
Attachment #78020 - Attachment is obsolete: true
Comment on attachment 78021 [details] [diff] [review]

>+    nsCOMPtr<nsIXMLDocument> xmlDoc = do_QueryInterface(mDocument);
>+    if (xmlDoc) {
>+      // We stopped supporting <style src="..."> for XHTML as it is
>+      // non-standard.
>+      *aIsInline = PR_TRUE;
>+      return;
>+    }

Don't check for xhtml-ness this way, do it by looking at the namespace in the
mNodeInfo which is kNameSpaceID_None for html and kNameSpaceID_HTML for xhtml.

other then that, r=sicking
Attachment #78021 - Flags: review+
Attached patch v1.1Splinter Review
Attachment #78021 - Attachment is obsolete: true
Comment on attachment 78083 [details] [diff] [review]

Moving forward r=sicking.
Attachment #78083 - Flags: review+
Comment on attachment 78083 [details] [diff] [review]

- In nsHTMLStyleElement.cpp:

+  if (mNodeInfo->NamespaceEquals(kNameSpaceID_HTML)) {

Make that kNameSpaceID_XHTML.

Attachment #78083 - Flags: superreview+
Comment on attachment 78083 [details] [diff] [review]

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #78083 - Flags: approval+
What can this break, and what testing have you done to ensure this lands without
Pls land this on the trunk to bake for a few days, have QA look, and make sure
there are no regressions introduced, then come back to us for approval - ADT.
Removing adt1.0.0. Pls renominate once it has been on the trunk a couple days,
and been tested. 
Keywords: adt1.0.0
*** Bug 136852 has been marked as a duplicate of this bug. ***
Severity: normal → major
Summary: Style ignored in this page → XHTML <style> element not working
Whiteboard: [Hixie-P1][Hixie-1.0] affects W3C Selectors test suite
Checked in to the trunk. Leaving open so we can renominate for branch and ADT.
Looks good.

Having this fix means we pass over 100 more tests on the W3C Selectors test
suite. Without this fix, XHTML is basically pointless.

VERIFIED FIXED on the trunk.
I've gone through all of the open bugs filed since this landed, none of them
seem to be caused by these changes. I've mailed asking if he
could do some additional testing like he did for bug 34849, no reponse yet.
Whiteboard: [Hixie-P1][Hixie-1.0] affects W3C Selectors test suite → [fixed on trunk][Hixie-P1][Hixie-1.0] affects W3C Selectors test suite
Whiteboard: [fixed on trunk][Hixie-P1][Hixie-1.0] affects W3C Selectors test suite → [fixed on trunk][Hixie-P1][Hixie-1.0][ADT1] affects W3C Selectors test suite
amar, could you test this?  It looks like this passes the Selector test.  What
else can be affected in html or xml and what test cases need to be run?
Attached file testcase: HTML style src (obsolete) —
The patch does not affect HTML. This testacase demonstrates it by showing that
the part of the patch that is outside of XML-only code is not affecting HTML
(style src should still work in HTML)
Sorry, wrong stylesheet link, this is the correct one.
adding adt1.0.0+.  Please checkin to the branch as soon as possible and add the
fixed1.0.0 keyword.
Keywords: adt1.0.0adt1.0.0+
*** Bug 139238 has been marked as a duplicate of this bug. ***
Testcases listed in this bug and related bugs passed,
Verified fix checked in cvs(rev 1.43)
Checked in to branch.
Closed: 23 years ago
Keywords: fixed1.0.0
Resolution: --- → FIXED
 I have tested HTML-Tables and Frames testcases which uses CSS for regressions
on trunk build 2002-04-14-11-trunk. All the testcases looks fine, no regressions.
And Hixie verified on trunk earlier, marking as such.
Checked this on win2000 build ID: 2002-05-22-1.0.0 branch 

I ran attachment 75083 [details], it is fixed on the branch.
The CSS selector testcases are also working fine.

Changing kweyword fixed1.0.0 to verified1.0.0
You need to log in before you can comment on or make changes to this bug.