Closed
Bug 481335
Opened 16 years ago
Closed 16 years ago
[FIX]Consider caching the nsIURI for the href in anchors
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Keywords: perf)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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?
I think the extra memory would be fine. Aren't we keeping a hash of uri->link in the document anyway?
Assignee | ||
Comment 2•16 years ago
|
||
Oh, hmm. Indeed. So this would just be something like a single word.
I'll do this tomorrow.
Assignee | ||
Comment 4•16 years ago
|
||
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.
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → bzbarsky
Summary: Consider caching the nsIURI for the href in anchors → [FIX]Consider caching the nsIURI for the href in anchors
Assignee | ||
Comment 5•16 years ago
|
||
Attachment #365452 -
Flags: superreview?(jst)
Attachment #365452 -
Flags: review?(jst)
Assignee | ||
Comment 6•16 years ago
|
||
Jonas, feel free to steal the review if you want, since a bunch of the patch is in nsAttrValue.
Assignee | ||
Comment 7•16 years ago
|
||
Oh, and the eFloatValue changes are just things that should have been added when that value type was added...
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)
Attachment #365452 -
Flags: superreview?(jst)
Attachment #365452 -
Flags: superreview+
Attachment #365452 -
Flags: review?(jst)
Attachment #365452 -
Flags: review+
Assignee | ||
Comment 9•16 years ago
|
||
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.
Assignee | ||
Comment 10•16 years ago
|
||
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.
Assignee | ||
Comment 11•16 years ago
|
||
Attachment #365452 -
Attachment is obsolete: true
Assignee | ||
Comment 12•16 years ago
|
||
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•