patch: resolve DOM src and href attributes against base
Maciej Stachowiak
mjs at apple.com
Mon Nov 3 03:11:07 CET 2003
On Nov 2, 2003, at 4:38 PM, Dirk Mueller wrote:
> On Thursday 30 October 2003 17:48, Maciej Stachowiak wrote:
>
>>> why did you remove the special-casing of <img src> ?
>> Retrieving the src attribute of <img src=""> from JavaScript returns
>> the base document URL from the src attribute in other browsers.
>
> No, only in IE.
That doesn't match my testing.
> Anyway, the <img src=""> case is not interesting, because it
> does not matter. it is very unlikely that your html document
> containing this
> construct would be a validly displayable image, so no webauthor would
> ever do
> that.
If there's one thing I've learned working on a browser, it's that web
authors do things "no web author would ever do" all the time.
> The point is, web authors want to distinguish the <img> case from the
> <img
> src=foo> case. Your patch breaks this, because you return the document
> base
> url for <img>. (where ATTR_SRC is a null string).
In Mozilla, this does indeed return the empty string and with my patch
it returns the base URL. I'll fix that.
>
>> tested and <img src> does the same, at least in Mozilla.
>
> Not in my Mozilla (1.4.0). anyway, the isEmpty/isNull difference is not
> important, because nobody is going to use <img src> anyway. But as
> Mozilla
> does not expand <img src=""> to the document base url, we did it this
> way.
I just tried this test case in Mozilla Firebird 0.7 (based on Mozilla
1.5):
-------------- next part --------------
An HTML attachment was scrubbed...
URL: https://mail.kde.org/mailman/private/khtml-devel/attachments/20031102/80e05731/src.html
-------------- next part --------------
The alert popped up with the file URL to the page, not the empty
string. What's your test case?
>>> its not as if we added that code just for fun..
>> Well I didn't take it out just for fun - I tested. Why did you add it?
>
> Its more important for HTMLScriptElement::src. where you just broke the
> difference between <script> and <script src>.
Good point. Here again, Mozilla distinguishes empty attribute value
from completely missing attribute.
Here's my patch that I think gets all of this 100% right.
-------------- next part --------------
Index: ChangeLog
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/ChangeLog,v
retrieving revision 1.2179
diff -u -p -r1.2179 ChangeLog
--- ChangeLog 2003/11/02 03:53:59 1.2179
+++ ChangeLog 2003/11/03 02:07:10
@@ -1,3 +1,24 @@
+2003-11-02 Maciej Stachowiak <mjs at apple.com>
+
+ Reviewed by SOMEBODY WHO CARES.
+
+ - don't resolve Null href or src attributes, since the distinction
+ between empty and absent attribute is important.
+
+ * khtml/dom/html_base.cpp:
+ (HTMLIFrameElement::src):
+ * khtml/dom/html_form.cpp:
+ (HTMLInputElement::src):
+ * khtml/dom/html_head.cpp:
+ (HTMLBaseElement::href):
+ (HTMLLinkElement::href):
+ (HTMLScriptElement::src):
+ * khtml/dom/html_image.cpp:
+ (HTMLAreaElement::href):
+ (HTMLImageElement::src):
+ * khtml/dom/html_inline.cpp:
+ (HTMLAnchorElement::href):
+
2003-11-01 Maciej Stachowiak <mjs at apple.com>
Reviewed by Darin.
Index: khtml/dom/html_base.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/dom/html_base.cpp,v
retrieving revision 1.5
diff -u -p -r1.5 khtml/dom/html_base.cpp
--- khtml/dom/html_base.cpp 2003/10/28 19:13:11 1.5
+++ khtml/dom/html_base.cpp 2003/11/03 02:07:10
@@ -371,7 +371,8 @@ DOMString HTMLIFrameElement::src() const
{
if(!impl) return DOMString();
DOMString s = ((ElementImpl *)impl)->getAttribute(ATTR_SRC);
- s = ownerDocument().completeURL( s );
+ if (!s.isNull())
+ s = ownerDocument().completeURL( s );
return s;
}
Index: khtml/dom/html_form.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/dom/html_form.cpp,v
retrieving revision 1.6
diff -u -p -r1.6 khtml/dom/html_form.cpp
--- khtml/dom/html_form.cpp 2003/10/28 19:13:11 1.6
+++ khtml/dom/html_form.cpp 2003/11/03 02:07:13
@@ -464,7 +464,8 @@ DOMString HTMLInputElement::src() const
{
if(!impl) return DOMString();
DOMString s = static_cast<ElementImpl*>(impl)->getAttribute(ATTR_SRC);
- s = ownerDocument().completeURL( s );
+ if (!s.isNull())
+ s = ownerDocument().completeURL( s );
return s;
}
Index: khtml/dom/html_head.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/dom/html_head.cpp,v
retrieving revision 1.7
diff -u -p -r1.7 khtml/dom/html_head.cpp
--- khtml/dom/html_head.cpp 2003/10/28 19:13:11 1.7
+++ khtml/dom/html_head.cpp 2003/11/03 02:07:13
@@ -60,7 +60,8 @@ DOMString HTMLBaseElement::href() const
{
if(!impl) return DOMString();
DOMString s = ((ElementImpl *)impl)->getAttribute(ATTR_HREF);
- s = ownerDocument().completeURL( s );
+ if (!s.isNull())
+ s = ownerDocument().completeURL( s );
return s;
}
@@ -139,7 +140,8 @@ DOMString HTMLLinkElement::href() const
{
if(!impl) return DOMString();
DOMString s = ((ElementImpl *)impl)->getAttribute(ATTR_HREF);
- s = ownerDocument().completeURL( s );
+ if (!s.isNull())
+ s = ownerDocument().completeURL( s );
return s;
}
@@ -385,7 +387,8 @@ DOMString HTMLScriptElement::src() const
{
if(!impl) return DOMString();
DOMString s = ((ElementImpl *)impl)->getAttribute(ATTR_SRC);
- s = ownerDocument().completeURL( s );
+ if (!s.isNull())
+ s = ownerDocument().completeURL( s );
return s;
}
Index: khtml/dom/html_image.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/dom/html_image.cpp,v
retrieving revision 1.8
diff -u -p -r1.8 khtml/dom/html_image.cpp
--- khtml/dom/html_image.cpp 2003/10/28 19:13:11 1.8
+++ khtml/dom/html_image.cpp 2003/11/03 02:07:13
@@ -97,7 +97,8 @@ DOMString HTMLAreaElement::href() const
{
if(!impl) return DOMString();
DOMString s = ((ElementImpl *)impl)->getAttribute(ATTR_HREF);
- s = ownerDocument().completeURL( s );
+ if (!s.isNull())
+ s = ownerDocument().completeURL( s );
return s;
}
@@ -289,7 +290,8 @@ DOMString HTMLImageElement::src() const
{
if(!impl) return DOMString();
DOMString s = ((ElementImpl *)impl)->getAttribute(ATTR_SRC);
- s = ownerDocument().completeURL( s );
+ if (!s.isNull())
+ s = ownerDocument().completeURL( s );
return s;
}
Index: khtml/dom/html_inline.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/WebCore/khtml/dom/html_inline.cpp,v
retrieving revision 1.5
diff -u -p -r1.5 khtml/dom/html_inline.cpp
--- khtml/dom/html_inline.cpp 2003/10/28 19:13:11 1.5
+++ khtml/dom/html_inline.cpp 2003/11/03 02:07:13
@@ -96,7 +96,8 @@ DOMString HTMLAnchorElement::href() cons
{
if(!impl) return DOMString();
DOMString s = ((ElementImpl *)impl)->getAttribute(ATTR_HREF);
- s = ownerDocument().completeURL( s );
+ if (!s.isNull()
+ s = ownerDocument().completeURL( s );
return s;
}
-------------- next part --------------
I looked at your tree and the only remaining problem seems to be that
it does not expand attributes that are empty rather than missing, I
suspect for at least <a href=""> this is going to matter on some sites.
(However, it looks like Script and IFrame were fixed just today... I
was about to say they weren't doing the completeURL at all and then I
did a fresh update.)
Would you like me to make a patch against khtml HEAD?
Thanks so much for pointing out the problem with my original patch.
Regards,
Maciej
More information about the Khtml-devel
mailing list