481335 - [FIX]Consider caching the nsIURI for the href in anchors

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

It would be nice if GetHrefURIForAnchors() could use a cached value.  We could clear it at the same time as we clear the link state...

This does mean a bit more memory usage, of course.  Thoughts?

Comment 1

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

I think the extra memory would be fine. Aren't we keeping a hash of uri->link in the document anyway?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, hmm.  Indeed.  So this would just be something like a single word.

I'll do this tomorrow.

Comment 3

[Brendan Eich [:brendan]]

Low hanging fruit, the very model of.

/be

Comment 4

[Boris Zbarsky [:bzbarsky]]

Not sure about that; there is a user-visible behavior change here in some cases due to our existing poor handling of base changes, so I'm loath to take on branch at this point.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Like so, say

Comment 6

[Boris Zbarsky [:bzbarsky]]

Jonas, feel free to steal the review if you want, since a bunch of the patch is in nsAttrValue.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Oh, and the eFloatValue changes are just things that should have been added when that value type was added...

Comment 8

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

Comment on attachment 365452 [details] [diff] [review]
Like so, say

Looks good. Would be nice with tests though. Specifically test that moving nodes around in a tree where there are xml:base works. Where works means that .href resolves to the correct thing twice. And that links get colored appropriately when a page is loaded (you can generate unique urls)

Comment 9

[Boris Zbarsky [:bzbarsky]]

I can pretty easily write tests for the xml:base thing that don't work, either with or without this patch (in terms of link coloring).  All it takes is xml:base in disconnected subtrees.

I'll write some basic tests, though.

Comment 10

[Boris Zbarsky [:bzbarsky]]

OK, I did write a test, and it in fact caught bug 482394.  I'll check in with some todo()s and fix in that bug.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: With test

Comment 12

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/dfee73c1165d