Closed Bug 47534 Opened 24 years ago Closed 21 years ago

[FIX]image.longdesc not an absolute URL if given as relative...

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.5alpha

People

(Reporter: jst, Assigned: bzbarsky)

References

Details

(Keywords: access, dom0)

Attachments

(1 file, 1 obsolete file)

As requested
50.24 KB, patch
caillon
: review+
jst
: superreview+
Details | Diff | Splinter Review
Mozilla doesn't expand the longdesc attribute on image elements to an absolute URL when the URL is accessed through the .longdesc property on the image element.
This also happens with (see http://www.w3.org/TR/html4/index/attributes.html)... - <IMG>, <FRAME>, <IFRAME> "longdesc" attribute - <INS>, <DEL>, <BLOCKQUOTE>, <Q> "cite" attribute - <FORM> "action" attribute - <SCRIPT>, <INPUT>, <FRAME>, <IFRAME> "src" attribute - <HEAD> "profile" attribute - <IMG>, <INPUT>, <OBJECT> "usemap" attribute (not sure this is even necessary, since I think it always refers to image maps in the current document anyway) - <OBJECT> "classid", "data", "archive" [space-separated list] (interpreted relative to "codebase" attribute of OBJECT, then BASE) - Deprecated: <APPLET> "code", "archive" [comma-separated list], "object" attributes (interpreted relative to "codebase" attribute of APPLET, then BASE) - Deprecated: <BODY> "background" attribute.
Thanks for the list Tim, unfortunately this will not make it into nsbeta3 (unless someone offers to fix this)... This bug has been marked "future" because the original netscape engineer working on this is over-burdened. If you feel this is an error, that you or another known resource will be working on this bug,or if it blocks your work in some way -- please attach your concern to the bug for reconsideration.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Keywords: access
Blocks: longdesc
Keywords: dom0
So.... _should_ these properties return absolute URIs? The DOM spec is kinda vague here....
Lest it be unclear, I'm volunteering to fix this bug, depending on what the DOM spec actually says.
Blocks: frame-longdesc
(Shouldn't component for this bug be "DOM HTML"?) Hmm, the spec is a bit vague. DOM level 2 describes HTMLImageElement.longDesc as: longDesc of type DOMString URI [IETF RFC 2396] designating a long description of this image or frame. See the longdesc attribute definition in HTML 4.01. Contrast this with the definition of HTMLAnchorElement.href which explicitly says the value is an absolute URI: href of type DOMString The absolute URI [IETF RFC 2396] of the linked resource. See the href attribute definition in HTML 4.01. HtmlImageElement.src and HtmlImageElement.longdesc are just URIs, whatever that is intended to mean. The HTML spec doesn't specify the attribute must be absolute (contrast with the href attribute def'n for <BASE>, which HTML4 says must be absolute). Although I know almost nothing about the DOM and am utterly unqualified to comment, I'd read it as saying the value is allowed to be a relative URI, in which case some other mechanism must be used to resolve it as an absolute URI. Unfortunately that would put an extra burden on the user to check the value and possibly convert it to an absolute URI.
Mass-reassigning bugs to dom_bugs@netscape.com
Assignee: jst → dom_bugs
Status: ASSIGNED → NEW
Taking bug, pending resolution of the question I raised on www-dom@w3.org
Assignee: dom_bugs → bzbarsky
Priority: P3 → P2
Target Milestone: Future → mozilla1.5beta
Mail on the list: On Wed, 2003-03-19 at 20:41, Johnny Stenback wrote: > My take on this is that it is an accidental difference in the spec. Same > goes for HTMLImageElement.src, existing implementations (maybe not all, > but at least the browsers) return that as an absolute URI as well. > > Philippe, should we clarify this in the spec at some point? Correct, an erratum seems in order. How about: HTMLDocument.referrer HTMLHeadElement.profile HTMLLinkElement.href HTMLBaseElement.href HTMLBodyElement.background HTMLQuoteElement.cite HTMLModElement.cite HTMLObjectElement.codeBase HTMLObjectElement.data HTMLAppletElement.codeBase HTMLAreaElement.href HTMLScriptElement.src HTMLFrameElement.longDesc HTMLFrameElement.src HTMLIFrameElement.longDesc HTMLIFrameElement.src Philippe --------------------------------------------------------- The approach I plan to take is to create an NS_IMPL_URI_ATTR macro which will automatically resolve the URI in the getter (to parallel our NS_IMPL_STRING_ATTR, etc macros). Seem reasonable?
yes, that is absolutly (no phun intended) the solution i would recommend. Remember to get the uri to resolve against from the current node so that you'd automatically get support for xml:base
> Remember to get the uri to resolve against from the current node Feh. We have GetBaseURL on nsIHTMLContent and GetXMLBaseURI on nsIXMLContent and life just sucks (two QIs plus, in the end, two calls to the doc to get the base URI is not called for). For example, GetXMLBaseURI is implemented by nsXMLElement, which means XHTML elements will not do anything with it, no? How about we just put GetBaseURI on nsIContent, move the impl of GetXMLBaseURI to nsGenericElement or something, have nsGenericHTMLElement override it to do the HTML base tag mess? These functions could also handle getting the base uri from the document... We could even leave the GetXMLBaseURI thing in place if we want, assuming someone ever needs the actual xml:base value instead of a real base URI. This would simplify some consumers who _should_ really be looking at the right base URIs but are not (eg <form>).
Yeah, yeah, yeah, and yeah. (well, I don't really know that anyone cares about GetXMLBaseURI(), really...)
Depends on: 209573
Attached patch Proposed patch (obsolete) — — Splinter Review
This works... I suppose I could have used NS_NewURIWithDocumentCharset, but then I would need to addref nsHTMLUtils for every single nsGenericHTMLElement, which seems silly. Thoughts?
Summary: image.longdesc not an absolute URL if given as relative... → [FIX]image.longdesc not an absolute URL if given as relative...
Target Milestone: mozilla1.5beta → mozilla1.5alpha
Comment on attachment 127133 [details] [diff] [review] Proposed patch reviews?
Attachment #127133 - Flags: superreview?(jst)
Attachment #127133 - Flags: review?(caillon)
Attachment #127133 - Flags: review?(caillon) → review+
How about moving NS_NewURIWithDocumentCharset into nsConteUtils and killing nsHTMLUtils once n' for all? There's nothing HTML about nsHTMLUtils, and it seems like a silly class, especially given how we use it. It should IMO die. If you'd do that, you could clean up all the code that now uses nsHTMLUtils, and make them use nsContentUtils in stead, w/o this silly AddRef/Release stuff. Thoughts?
Sure. I could do that.
Attached patch As requested — — Splinter Review
No more nsHTMLUtils. The diff does not include those two files, but they should be cvs removed when checking in.
Attachment #127133 - Attachment is obsolete: true
Attachment #127133 - Flags: superreview?(jst)
Attachment #127186 - Flags: superreview?(jst)
Attachment #127186 - Flags: review?(caillon)
Comment on attachment 127186 [details] [diff] [review] As requested Nice! One nit: - In nsHTMLAreaElement::SetAttr(): if (aName == nsHTMLAtoms::accesskey && aNameSpaceID == kNameSpaceID_None) { RegUnRegAccessKey(PR_FALSE); + } + + if (aName == nsHTMLAtoms::href && kNameSpaceID_None == aNameSpaceID) { aNameSpaceID == kNameSpaceID_None here too, to match the above line, if nothing else... sr=jst
Attachment #127186 - Flags: superreview?(jst) → superreview+
Comment on attachment 127186 [details] [diff] [review] As requested I just had a look at the nsContentUtils portion of the patch since I already reviewed the rest. I'm cool with removing nsHTMLUtils and trust that it's done properly. >+ /** >+ * Create a new URI object from aSpec, using aBaseURI as the base. >+ * The charset associated to the new URI object will be the document >+ * character set of aDocument. >+ */ Comment nits: s/URI object/nsIURI/g ? 'charset' and 'character set' -- pick one and stick with it? ;-) Other than that, r=caillon
Attachment #127186 - Flags: review?(caillon) → review+
Made those changes, and checked in. jst, could you push through that spec erratum, by any chance?
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Notice I used the global modifier on the substitution. You forgot one of the URI Object -> nsIURI updates. :-)
Oops, sorry. Checked in that comment update.
Blocks: 213701
*** Bug 72524 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: