Closed
Bug 169124
Opened 23 years ago
Closed 23 years ago
xslt with xhtml namespace output is not navigable
Categories
(Core :: XSLT, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: jugsjagan, Assigned: sicking)
Details
Attachments
(4 files, 3 obsolete files)
|
3.16 KB,
application/x-zip-compressed
|
Details | |
|
368 bytes,
text/xml
|
Details | |
|
179 bytes,
text/xml
|
Details | |
|
27.53 KB,
patch
|
harishd
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
when an xslt outputs with html tag defined as
<html xmlns="http://www.w3.org/1999/xhtml">
the output appears but the input fields are not editable and javascript does not
work.
|
Assignee | |
Comment 2•23 years ago
|
||
please read www.mozilla.org/projects/xslt for info on how to attach testcases
|
|
Assignee | |
Updated•23 years ago
|
Attachment #99482 -
Attachment mime type: application/octet-stream → application/x-zip-compressed
|
|
Assignee | |
Comment 3•23 years ago
|
||
the problem is probably that nsHTMLInputElement is a leaf-element and thus can't
have any children. The fix for this is probably the same as for bug XXX; making
sure that we create generic XML elements for the contents of loaded stylesheets
Status: UNCONFIRMED → NEW
Ever confirmed: true
|
|
Assignee | |
Comment 4•23 years ago
|
||
|
|
Assignee | |
Comment 5•23 years ago
|
||
|
|
Assignee | |
Comment 6•23 years ago
|
||
Currently the XSL sink creates "real" XHTML elements for elements in the XHTML
namespace. This is bad since those objects can't always contain childnodes.
This is something that XSLT obviously needs since for stylesheets like
<xhtml:input>
<xsl:attribute>...</xsl:attribute>
</xhtml:input>
So this patch restructures the XML contentsink so that it's easy for a subclass
to compleatly override the creation and initialization of all elements. This is
done by overriding the CreateElement and CloseElement functions. These
functions return the created element and a flag indicating if the element
should be inserted in the tree. They *must* return opposite values for the
insertion-flag for any given element. Currently the only element that uses this
flag is xhtml:script. (recognize the design from the outputhandlers huh :) )
So this patch makes us create generic xml-elements for all elements,
undependent of namespace or localname.
However this patch only fixes stylesheets loaded through the xsl-sink, which is
only the case for the first stylesheet, not subsequent included or imported
stylesheets (or documents loaded through the document() xpath function). I'm
not really sure how to handle that case, but this patch should at least solve
90% of the problem.
|
||
Comment 7•23 years ago
|
||
Comment on attachment 103451 [details] [diff] [review]
patch to fix
this patch doesn't apply anymore :-(
Hrm. needs-work is gone, so I emulate that with -
Attachment #103451 -
Flags: review-
|
|
Assignee | |
Comment 8•23 years ago
|
||
Attachment #103451 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•23 years ago
|
Attachment #107267 -
Flags: superreview?(peterv)
Attachment #107267 -
Flags: review?(axel)
|
|
Assignee | |
Updated•23 years ago
|
Attachment #107267 -
Flags: review?(axel) → review?(harishd)
|
||
Comment 10•23 years ago
|
||
Comment on attachment 107267 [details] [diff] [review]
sync with tip
>Index: xml/document/src/nsXMLContentSink.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/content/xml/document/src/nsXMLContentSink.cpp,v
>retrieving revision 1.240
>diff -u -r1.240 nsXMLContentSink.cpp
>--- xml/document/src/nsXMLContentSink.cpp 22 Nov 2002 07:39:50 -0000 1.240
>+++ xml/document/src/nsXMLContentSink.cpp 24 Nov 2002 05:37:29 -0000
>@@ -599,36 +599,141 @@
>+ NS_ENSURE_SUCCESS(rv, rv);
> elementFactory->CreateInstanceByTag(aNodeInfo, aResult);
Please put a newline after NS_ENSURE_* (also in other places)
>+ else if (tagAtom == nsHTMLAtoms::title) {
>+ if (mTitleText.IsEmpty())
>+ mInTitle = PR_TRUE; // The first title wins
Add braces around this line
>+nsresult
>+nsXMLContentSink::CloseElement(nsIContent* aContent, PRBool* aAppendContent)
...
>+ else if (tagAtom == nsHTMLAtoms::base) {
>+ if (mBaseElement) {
>+ rv = ProcessBASETag();
Do we really want to call ProcessBASETag for every base element? (I know
mBaseElement is only set once)
>+ return rv;
>+}
>+
>+
>+
Don't add too much whitespace.
>+ // For XSL, we need to wait till after the transform
XSLT :-P
>+ // to set the root content object.
>+ if (!mXSLTransformMediator)
>+ mDocument->SetRootContent(mDocElement);
Add braces and deindent (file uses 2 spaces)
>+ // Set the ID attribute atom on the node info object for this node
>+ if ((aIndex != (PRUint32)-1) && NS_SUCCEEDED(result)) {
>+ nsCOMPtr<nsIAtom> IDAttr =
>+ dont_AddRef(NS_NewAtom((const PRUnichar *)aAtts[aIndex]));
Use do_GetAtom
>Index: xml/document/src/nsXMLContentSink.h
>@@ -215,6 +214,7 @@
> nsCOMPtr<nsIHTMLContent> mBaseElement;
> nsCOMPtr<nsIHTMLContent> mMetaElement;
> nsCOMPtr<nsIHTMLContent> mLinkElement;
>+ nsCOMPtr<nsIContent> mNonAddedContent;
You don't use this anywhere
|
|
Assignee | |
Comment 11•23 years ago
|
||
This fixes all comments from peterv. I also removed mBaseElement and
mMetaElement and used arguments instead. And I removed mLinkElement and
ProcessLINKTag since they weren't used.
Attachment #107267 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•23 years ago
|
Attachment #107473 -
Flags: superreview?(peterv)
Attachment #107473 -
Flags: review?(harishd)
|
||
Comment 12•23 years ago
|
||
Comment on attachment 107473 [details] [diff] [review]
fixes petervs comments
>- if (aNameSpaceID == nsXULAtoms::nameSpaceID) {
>+ if (aNodeInfo->NamespaceEquals(nsXULAtoms::nameSpaceID)) {
>+ *aAppendContent = PR_TRUE;
> nsXULPrototypeElement* prototype = new nsXULPrototypeElement();
> if (!prototype)
> return NS_ERROR_OUT_OF_MEMORY;
>@@ -748,18 +747,19 @@
> return NS_OK;
> }
> else
>- return nsXMLContentSink::CreateElement(aAtts, aAttsCount, aNameSpaceID, aNodeInfo, aResult);
>+ return nsXMLContentSink::CreateElement(aAtts, aAttsCount, aNodeInfo,
>+ aLineNumber, aResult,
>+ aAppendContent);
> }
Loose the 'else'.
> nsresult
> nsXBLContentSink::AddAttributes(const PRUnichar** aAtts,
>- nsIContent* aContent,
>- PRBool aIsHTML)
>+ nsIContent* aContent)
> {
> if (aContent->IsContentOfType(nsIContent::eXUL))
> return NS_OK; // Nothing to do, since the proto already has the attrs.
> else
>- return nsXMLContentSink::AddAttributes(aAtts, aContent, aIsHTML);
>+ return nsXMLContentSink::AddAttributes(aAtts, aContent);
> }
Loose the 'else'.
>Index: xml/document/src/nsXMLContentSink.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/content/xml/document/src/nsXMLContentSink.cpp,v
>retrieving revision 1.240
>diff -u -r1.240 nsXMLContentSink.cpp
>--- xml/document/src/nsXMLContentSink.cpp 22 Nov 2002 07:39:50 -0000 1.240
>+++ xml/document/src/nsXMLContentSink.cpp 26 Nov 2002 15:45:37 -0000
>@@ -191,6 +191,7 @@
> mPrettyPrintXML = PR_TRUE;
> mPrettyPrintHasSpecialRoot = PR_FALSE;
> mPrettyPrintHasFactoredElements = PR_FALSE;
>+ mHasProcessedBase = PR_FALSE;
> }
optional:
How about an unsigned int flag to replace all the packed bools? That is
something like:
PRUint16 mFlags; and mFlags can contain
NS_XML_SINK_FLAG_CONSTRAIN_SIZE
NS_XML_SINK_FLAG_IN_TITLE
NS_XML_SINK_FLAG_....
NS_XML_SINK_FLAG_HAS_PROCESSED_BASE.
>- // Create the content element using the element factory.
>+
>+ PRInt32 nameSpaceID;
>+ aNodeInfo->GetNamespaceID(nameSpaceID);
Needs null checking - How about adding NS_ENSURE_ARG_POINTER at the begining of
the method?
>+ nsCOMPtr<nsIAtom> tagAtom;
>+ aNodeInfo->GetNameAtom(*getter_AddRefs(tagAtom));
Missing null check. Please add NS_ENSURE_ARG_POITNER.
>+
>+nsresult
>+nsXMLContentSink::CloseElement(nsIContent* aContent, PRBool* aAppendContent)
>+{
>+ *aAppendContent = PR_FALSE;
>+ if (!aContent->IsContentOfType(nsIContent::eHTML)) {
>+ return NS_OK;
>+ }
Add NS_ENSURE_ARG_POINTER.
>+nsXMLContentSink::ProcessBASETag(nsIContent* aContent)
> {
....
>+ if (aContent->GetAttr(kNameSpaceID_None, nsHTMLAtoms::target, value) ==
>+ NS_CONTENT_ATTR_HAS_VALUE) {
> mDocument->SetBaseTarget(value);
> }
Add NS_ENSURE_ARG_POINTER :-). Please do this whereever required.
> nsCOMPtr<nsIAtom> fieldAtom(dont_AddRef(NS_NewAtom(header)));
>- rv=ProcessHeaderData(fieldAtom,result,mMetaElement);
>+ rv=ProcessHeaderData(fieldAtom, result, aContent);
Add whitespace around '='.
>+ nsCOMPtr<nsIContent> parent = getter_AddRefs(GetCurrentContent());
>
>- PushContent(content);
>+ parent->AppendChildTo(content, PR_FALSE, PR_FALSE);
Please add a null check just to be on the safe side or add
NS_ENSURE_TRUE(parent, NS_ERROR_UNEXPECTED).
>+ nsCOMPtr<nsIAtom> IDAttr = do_GetAtom((const PRUnichar *)aAtts[aIndex]);
I don't think the casting is required. Could you please double check that?
>+ else if (appendContent) {
>+ nsCOMPtr<nsIContent> parent = getter_AddRefs(GetCurrentContent());
>....
>+ parent->AppendChildTo(content, PR_FALSE, PR_FALSE);
Missing null check.
|
|
Assignee | |
Comment 13•23 years ago
|
||
This fixes all comments from Harish except two things: I didn't add
NS_ENSURE_ARG_POINTER for the arguments to different functions since those
functions should never be called with null-values (and i checked that they
arn't), added assertions instead. And i didn't combine the flags into a bitmap
since we never have very many of these objects alive so i'd rather keep the
code clean then save a couple of bytes of runtime memory
Attachment #107473 -
Attachment is obsolete: true
|
|
||
Comment 14•23 years ago
|
||
Comment on attachment 107868 [details] [diff] [review]
fixes harishs comments
>+nsresult
>+nsXMLContentSink::CloseElement(nsIContent* aContent, PRBool* aAppendContent)
>+{
>+ NS_ASSERTION(aContent, "missing element to close");
>+
>+ *aAppendContent = PR_FALSE;
>+ if (!aContent->IsContentOfType(nsIContent::eHTML)) {
>+ return NS_OK;
>+ }
>+
>+ nsCOMPtr<nsIAtom> tagAtom;
>+ aContent->GetTag(*getter_AddRefs(tagAtom));
>+
>+ nsresult rv = NS_OK;
>+
>+ if (tagAtom == nsHTMLAtoms::script) {
>+ rv = ProcessEndSCRIPTTag(aContent);
>+ *aAppendContent = PR_TRUE;
>+ }
>+ else if (tagAtom == nsHTMLAtoms::title) {
>+ if (mInTitle) { // The first title wins
Combine the ifs
>+ nsCOMPtr<nsIDOMNSDocument> dom_doc(do_QueryInterface(mDocument));
>+ if (dom_doc) {
>+ mTitleText.CompressWhitespace();
>+ dom_doc->SetTitle(mTitleText);
>+ }
>+ mInTitle = PR_FALSE;
>+ }
>+ }
>+ else if (tagAtom == nsHTMLAtoms::base) {
>+ if (!mHasProcessedBase) { // The first title wins
s/title/base/
Combine the ifs
>+ rv = ProcessBASETag(aContent);
>+ mHasProcessedBase = PR_TRUE;
>+ }
>+ }
>+ else if (tagAtom == nsHTMLAtoms::meta) {
>+ rv = ProcessMETATag(aContent);
>+ }
>+ else if (tagAtom == nsHTMLAtoms::link || tagAtom == nsHTMLAtoms::style) {
>+ nsCOMPtr<nsIStyleSheetLinkingElement> ssle(do_QueryInterface(aContent));
>+
>+ if (ssle) {
>+ ssle->SetEnableUpdates(PR_TRUE);
>+ rv = ssle->UpdateStyleSheet(nsnull, mStyleSheetCount);
>+ if (NS_SUCCEEDED(rv) || (rv == NS_ERROR_HTMLPARSER_BLOCK)) {
>+ if (rv == NS_ERROR_HTMLPARSER_BLOCK && mParser) {
>+ mParser->BlockParser();
>+ }
Remove one space.
>+ mStyleSheetCount++;
++mStyleSheetCount
>+ }
>+ }
>+ }
>+
>+ return rv;
>+}
>+
Moved r=harishd.
Attachment #107868 -
Flags: superreview+
Attachment #107868 -
Flags: review+
|
|
||
Comment 15•23 years ago
|
||
Comment on attachment 107868 [details] [diff] [review]
fixes harishs comments
Removing non-existing r=harish. My bad.
Attachment #107868 -
Flags: review+ → review?(harishd)
|
|
Assignee | |
Comment 16•23 years ago
|
||
Comment on attachment 107868 [details] [diff] [review]
fixes harishs comments
last fixes made locally. Removing the review+ since i'm not sure where you
moved it from ;-)
|
|
||
Comment 17•23 years ago
|
||
Comment on attachment 107868 [details] [diff] [review]
fixes harishs comments
r=harishd
Attachment #107868 -
Flags: review?(harishd) → review+
|
|
Assignee | |
Comment 18•23 years ago
|
||
checked in, thanks for reviews
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
|
||
Updated•23 years ago
|
Attachment #107267 -
Flags: superreview?(peterv)
Attachment #107267 -
Flags: review?(harishd)
|
|
||
Updated•23 years ago
|
Attachment #107473 -
Flags: superreview?(peterv)
Attachment #107473 -
Flags: review?(harishd)
You need to log in
before you can comment on or make changes to this bug.
Description
•