325404 - Get rid of _base_href and _base_target attributes

Status: RESOLVED FIXED

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

Description

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

We should use properties rather then attributes for this. Patch coming up.

Comment 1

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

Attachment: Patch to fix

I'm not sure we really need the security check, but i'd rather not remove it in this patch.

I made cloning work since it wasn't a whole lot of work.

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 210315 [details] [diff] [review]
Patch to fix

Looks good, r+sr=jst

Comment 3

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

Attachment: testcase

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

+  if (HasProperties() && (prop = GetProperty(nsHTMLAtoms::htmlBaseHref))) {
+    NS_STATIC_CAST(nsIAtom*, prop)->ToString(aBaseTarget);

not htmlBaseTarget?

 nsHTMLFragmentContentSink::ProcessBaseTag(nsIContent* aContent)
 {
   nsAutoString value;
   if (aContent->GetAttr(kNameSpaceID_None, nsHTMLAtoms::href, value)) {
-    mBaseHREF = value;
+    nsCOMPtr<nsIURI> baseHrefURI;
+    nsresult rv = NS_NewURI(getter_AddRefs(baseHrefURI), value, nsnull);

shouldn't this pass the document's charset and URI too?

Comment 5

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

Attachment: Fix comments from biesi

Comment 6

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

Attachment: Updated testcase

Thanks for those findings. I've updated the testcase to make sure target works as it should too.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

> shouldn't this pass the document's charset and URI too?

what about the URI part? or are relative base uris not something that's supposed to work?

Comment 8

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

Yes, the <base href> attribute is an absolute uri.

http://www.w3.org/TR/html4/struct/links.html#h-12.4

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

ah, ok. so it differs from xml:base.

Comment 10

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

yes

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 210380 [details] [diff] [review]
Fix comments from biesi

r+sr=jst

Comment 12

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

Checked in

Comment 13

[Anne (:annevk)]

In HTML 5 it currently has to be resolved:
 <http://whatwg.org/specs/web-apps/current-work/#the-base>

Comment 14

[Hixie (not reading bugmail)]

Note that HTML5, mostly following the precedent set by IE7, makes <base href> and <base target> only work based on the first <base> element.

I did some studies and it seemed to me that the only sites this affected were automatically generated spammy link aggregators, so I doubt this will break any actually used pages.