Closed
Bug 209566
Opened 21 years ago
Closed 21 years ago
remove duplicated GetHrefCString code
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla1.5alpha
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Whiteboard: [patch])
Attachments
(1 file)
30.72 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
I just wrote a patch to remove some code duplication of GetHrefCString methods that I inflicted on the tree a number of years ago. While I was there, I noticed a vestigial |aOuter| parameter on HandleDOMEventForAnchors (which is used by the same three classes). Patch coming.
Assignee | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [patch]
Target Milestone: --- → mozilla1.5alpha
Assignee | ||
Comment 1•21 years ago
|
||
Oh, and while I was there I noticed it was copied to nsHTMLStyleElement.cpp, where it was incorrectly used in an ASCIItoUCS2 conversion (it's always returned UTF8). So I fixed the duplication there, too, fixed the conversion to be UTF8toUCS2, and renamed the function (s/CString/UTF8/). And since I was renaming the function I fixed the signature from *& to **.
Assignee | ||
Comment 2•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Attachment #125762 -
Flags: superreview?(jst)
Attachment #125762 -
Flags: review?(jst)
Assignee | ||
Comment 3•21 years ago
|
||
I should probably add a comment to nsGenericHTMLElement.h that callers should |AddRef| and |Release| nsHTMLUtils...
Comment 4•21 years ago
|
||
Comment on attachment 125762 [details] [diff] [review] patch r+sr=jst, though I never understood why this doesn't use an nsAString& out param in stead of char**... Wanna change that too while you're at it?
Attachment #125762 -
Flags: superreview?(jst)
Attachment #125762 -
Flags: superreview+
Attachment #125762 -
Flags: review?(jst)
Attachment #125762 -
Flags: review+
Comment 5•21 years ago
|
||
I'm still considering making it a nsIURI out param, since in 99% of cases we 1) Use an nsIURI to produce the string and 2) Use an nsIURI to normalize the string (inside nsILinkHandler) That's sorta what bug 166996 is about...
Comment 6•21 years ago
|
||
Hmm, yeah, that would make sense.
Assignee | ||
Comment 7•21 years ago
|
||
The original reason for char** was extreme sensitivity to performance -- this is called for every link in a document when we're loading a page. I'd rather not mess with it in this patch because I've put enough in here already, and also because bz seems to have some other ideas. Fix checked in to trunk, 2003-06-17 09:22 -0700.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•21 years ago
|
||
libgklayout.so Total: -1136 (+2953/-4089) Code: -1104 (+2912/-4016) Data: -32 (+41/-73) -32 (+41/-73) R (DATA) -32 (+41/-73) UNDEF:libgklayout.so:R +41 textCssStr.5542 -32 kCharsetAliasCID -41 textCssStr.5557 -1104 (+2912/-4016) T (CODE) -1104 (+2912/-4016) UNDEF:libgklayout.so:T +2360 nsGenericHTMLElement::HandleDOMEventFo +348 nsGenericHTMLElement::GetHrefUTF8ForAn +40 nsHTMLAnchorElement::GetHrefUTF8 +40 nsHTMLAreaElement::GetHrefUTF8 +40 nsHTMLLinkElement::GetHrefUTF8 +28 virtual function thunk (delta:-32) for +28 virtual function thunk (delta:-36) for +28 virtual function thunk (delta:-40) for -4 atexit -24 nsHTMLLinkElement::GetHref -28 virtual function thunk (delta:-40) for -28 virtual function thunk (delta:-36) for -28 virtual function thunk (delta:-32) for -28 nsHTMLStyleElement::GetStyleSheetURL -344 nsHTMLStyleElement::GetHrefCString -344 nsHTMLLinkElement::GetHrefCString -348 nsHTMLAreaElement::GetHrefCString -348 nsHTMLAnchorElement::GetHrefCString -2492 nsGenericHTMLElement::HandleDOMEventFo
You need to log in
before you can comment on or make changes to this bug.
Description
•