Closed Bug 72522 Opened 19 years ago Closed 19 years ago

support the baseURI attribute in XML and HTML DOM

Categories

(Core :: DOM: Core & HTML, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla0.9.1

People

(Reporter: sicking, Assigned: jst)

References

Details

(Whiteboard: [HAVE FIX])

Attachments

(4 files)

The Core DOM L3 attribute baseURI could simplify much code in mozilla a great 
deal. The attribute should support xml:base for XML DOM and <base> for HTML DOM 
(<base> should probobly affect the documents baseURI). I'm not really sure how 
XHTML should be handled but i'm guessing both xml:base and <html:base> affects 
baseURI in XHTML DOM.

This is a tracker so that i'll have someplace to add dependencies
This is not just a tracker, this is a real rfe. I had not made this a bug
earlier since I was going to fix this as part of another bug that required this
feature. Since we have more dependences it made sense to open this one; thanks.

Added dependences:

* Bug 48217 - XML Base should affect Open Link in New Window
* Bug 54074 - XML Base should affect :visited etc CSS styles
* Bug 49462 - XML Base should affect PI href

There are some difficulties in implementing DOM Level 3 stuff in Mozilla because
we do not want/cannot extend the existing DOM interfaces. We must create new
interfaces for DOM Level 3, and adopt code to QI for that etc. (will work just
fine for JS because of automatic interface flattening). There are some questions
to be answered, like should DOM Level 3 interfaces inherit from earlier
interfaces, or should they stand on their own (and implementations should
inherit from both). Currently I am leaning towards inheriting from old. Also we
need to choose a naming convention. I'll give the two options using Node as an
example: nsIDOM3Node or nsIDOMNode3.

Once the level 3 interface are in place, we need to figure out a way to hook
them to the implementations. At the moment I do not know how to do this.
Blocks: 48217, 49462, 54074
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
Uh huh, 54074 should have been 54047:

* Bug 54047 - XML Base should affect :visited etc CSS styles
Blocks: 54047
No longer blocks: 54074
Priority: -- → P3
This is the first implementation that actually compiled, so there are probably
going to be some changes to it. It does seem to work with some of my testcases
in XML at least. I have not implemented this for XUL.
Priority: P3 → P2
Whiteboard: [fixinhand]
I found one problem with the first patch: it did not check the current element's
baseURI, it checked the parent's. I fixed that in the proposed fix 1. That patch
is also cleaner as I updated my tree today and cleared some junk.

I'll come up with some testcases for this as well.
A couple of comments about the patch:

First of all, the long term solution for supporting DOM Level 3
interfaces/methods in mozilla should be (if we ever wanto freeze our DOM
interfaces, and I think we really wanto do that) to add interfaces (i.e.
nsIDOMNode3 or nsIDOM3Node) and implement those as tearoffs on the objects that
implement them (unless the object is a document or somesuch where the number of
vtables doesn't matter). But given that I'm about to rewrite the DOM interfaces
in XPIDL I'm ok with checking this in for now, but I will change this later
(i.e. I will create the new nsIDOMNode3, or whatever, interface).

This code:

  aURI.Assign(NS_ConvertASCIItoUCS2(spec));

can be written more efficiently as:

  CopyASCIItoUCS2(nsLiteralCString(spec), aURI);

In nsGenericElement::GetBaseURI()

...
+    nsCOMPtr<nsIContent> 
node(do_QueryInterface(NS_STATIC_CAST(nsIContent*,this)));
^^^^ Reanme this to 'content', when I see 'node' I think of the DOM interface,
not nsIContent.

+    while (node) {
+      nsCOMPtr<nsIXMLContent> xmlContent(do_QueryInterface(node));
+      if (xmlContent) {
+        xmlContent->GetXMLBaseURI(getter_AddRefs(uri));
+        break;
+      }
+      node->GetParent(*getter_AddRefs(node));

This will fail to work on many compilers (null ptr reference since
getter_AddRefs() nulls out 'node'), change this to:

       nsIContent *tmp = node; (or 'content' :-)
       tmp->GetParent(*getter_AddRefs(node));

+    }

Also in nsGenericElement::GetBaseURI():

+  if (!uri && mDocument) {
+    // HTML document or for some reason there was no XML content in XML document
+    mDocument->GetBaseURL(*getter_AddRefs(uri));
+  }

AFAIK there's no guarantee that nsIDocument::GetBaseURL() always returns a url,
I think the base url is only returned if it's actually specified in the
document, check for a null url and call nsIDocument::GetDocumentURL(). Same goes
for nsHTMLDocument::GetBaseURI(), mBaseURI can be null, AFAIK.

The method nsIXMLContent::GetXMLBaseURI() really belongs in nsIContent, we're
trying to eliminate nsIXMLContent so no new methods should be put in that
interface, but since the code for this method already exists in nsXMLElement I'd
accept this for now, it will be moved (and probably renamed) once nsIXMLContent
is eliminated, unless you wanto do it now.

Other than that the changes look good to me, sr=jst with the above changes.
In proposed fix 2 I fixed all the points jst talked about. I do believe that
base URL would always be in a document - if no base tag is present then it would
just be document base. But I bullet proofed this as well as jst noted.

Additionally, I changed nsXMLElement::GetXMLBaseURI() a little so that it also
tries first baseURL from document and if not present, the document URL. Also
because the method was changed to an interface I changed the signature to follow
that, and simplified the null check in the beginning a little. I also added some
baseURI stuff to XUL, which is better than nothing (basically just get document
URL).

The patch also contains some changes to nsIDOMDocument. When the dir property
was added I think they did the changes by hand, not with the xpidlc compiler as
they should have. The changes in the interface were made by the compiler.
r=harishd for the above patch.
sr=jst
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Reopening. Had to back out to fix binary compatibility, will reassign to jst
'cos he has the fix on the DOM conversion branch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla0.9 → mozilla0.9.1
You have the fix on the DOM conversion branch, reassigning.
Assignee: heikki → jst
Status: REOPENED → NEW
Look at bug 76641 to see what needs to go back once you fix this.
Whiteboard: [fixinhand] → [fixinhand][XPCDOM]
Whiteboard: [fixinhand][XPCDOM] → [HACE FIX]
Fixed.
Status: NEW → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Whiteboard: [HACE FIX] → [HAVE FIX]
Updating QA contact to Shivakiran Tummala.
QA Contact: desale → stummala
i created a small testcase to set the baseURI .... i was unable to set the
baseURI...there were no exceptions thrown... 
tested on linux and windows..

build id linux 2001-06-07-13-0.9.1
build id windows 2001-06-07-06-0.9.1

i think this can be reproduced on all operating systems....

<script>
document.write(document.documentElement.baseURI);
document.documentElement.setBaseURI("http://localhost");
document.write(document.documentElement.baseURI);
</script>

i am reopening this bug 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
there is no such method as setBaseURI. baseURI is an readonly attribute so 
first of all you can't set it, and second the syntax would be:

docuent.documentElement.baseURI = "http://foobar.com/"

which should throw an exception.
Status: REOPENED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Jonas, docuent.documentElement.baseURI = "http://foobar.com/" should *not* throw
an exception. It does so in mozilla, and that won't change, but by the
ECMAScript standard setting readonly properties on an object fails silently. The
DOM ECMAScript bindings doesn't specify otherwise so technically mozilla is
doing the wrong thing.

verified ...the exception was thrown when trying to set the baseURI
Status: RESOLVED → VERIFIED
the exception was thrown when trying to set the baseURI.......the script did not
die silently with out raising exception (johnny can u comment on this  .)
reopening
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
jst's comment previously was that we know that trying to set baseURI throws in
Mozilla and it won't change. Besides, that is a different bug if you want to
file one. Marking this fixed once more.
Status: REOPENED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Component: DOM: Core → DOM: Core & HTML
QA Contact: stummala → general
You need to log in before you can comment on or make changes to this bug.