209566 - remove duplicated GetHrefCString code

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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 **.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I should probably add a comment to nsGenericHTMLElement.h that callers should
|AddRef| and |Release| nsHTMLUtils...

Comment 4

[Johnny Stenback (:jst)]

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?

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

[Johnny Stenback (:jst)]

Hmm, yeah, that would make sense.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

  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