Closed Bug 542632 Opened 10 years ago Closed 10 years ago

Protect nsGenericHTMLElement::GetHrefURIForAnchors

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a2

People

(Reporter: sdwilsh, Assigned: sdwilsh)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch v1.0 (obsolete) — Splinter Review
We should protect it so consumers no longer access it.  This also makes the dns prefetch code use the cached URI.
Attachment #423857 - Flags: review?(bzbarsky)
>+      Link *link = static_cast<Link *>(static_cast<nsISupports *>(content.get()));

Why is this cast ok?
Because we only ever add Link elements to this queue, which are stored as weak references.  We QI the weak reference to nsIContent, so we can test if it is in the document (and exists).  We already know it's a Link for sure (we added it), so we have to cast down to nsISupports since that is what Link inherits from, and then cast to Link.
> so we have to cast down to nsISupports since that is what Link inherits from,
> and then cast to Link

But the nsISupports that nsIContent inherits from (and casts to) is not the same as the nsISupports Link inherits from, is it?

I would fully expect this code to either crash or behave really oddly if actually executed.
Bah, that's right.  What's the right way to get to Link in this case?  Do I have to add an IID for it to QI to?
That seems like the simplest way to go...
Darn, OK.  New patch soon.
I'm actually not sure how to do this QI and not destroy the other QI's.  Thoughts?
Destroy in what sense?
Nevermind.  I wasn't thinking.  The classes that implement Link do the QI implementation.
Attached patch v1.1 (obsolete) — Splinter Review
I tried making that far harder than it had to be...
Attachment #423857 - Attachment is obsolete: true
Attachment #426369 - Flags: review?(bzbarsky)
Attachment #423857 - Flags: review?(bzbarsky)
Comment on attachment 426369 [details] [diff] [review]
v1.1

Build finished, but this patch isn't right.  Arg (startup crash dereferencing a null pointer)
Attachment #426369 - Attachment is obsolete: true
Attachment #426369 - Flags: review?(bzbarsky)
Attached patch v1.2Splinter Review
OK, this one actually runs.
Attachment #426386 - Flags: review?(bzbarsky)
Comment on attachment 426386 [details] [diff] [review]
v1.2

>+++ b/content/html/content/src/nsHTMLAnchorElement.cpp
>   NS_HTML_CONTENT_INTERFACE_TABLE4(nsHTMLAnchorElement,

Needs to become TABLE5.

r=bzbarsky
Attachment #426386 - Flags: review?(bzbarsky) → review+
(In reply to comment #13)
> Needs to become TABLE5.
Fixed locally.
http://hg.mozilla.org/mozilla-central/rev/2777f7baaaad
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a2
Version: unspecified → Trunk
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.