EncodingUtils should have a deCOMtaminated replacement for GetUnicodeDecoderRaw

RESOLVED FIXED in mozilla28

Status

()

defect
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: hsivonen, Assigned: hsivonen)

Tracking

unspecified
mozilla28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

We could make code cleaner and less confusing if after EncodingUtils::FindEncodingForLabel we called something like
already_AddRefed<nsIUnicodeDecoder> EncodingUtils::DecoderForEncoding(const nsACString& aEncoding); instead of dealing with the nsICharsetConverterManager service.
Summary: EncodingUtils should have a deCOMtamineted replacement for GetUnicodeDecoderRaw → EncodingUtils should have a deCOMtaminated replacement for GetUnicodeDecoderRaw
Assignee: nobody → hsivonen
Comment on attachment 8337699 [details] [diff] [review]
Add deCOMtaminated factories

Review of attachment 8337699 [details] [diff] [review]:
-----------------------------------------------------------------

::: intl/uconv/idl/nsICharsetConverterManager.idl
@@ +23,5 @@
>  interface nsIUTF8StringEnumerator;
>  
>  /**
> + * DON'T ADD NEW USES OF THIS INTERFACE TO MOZILLA-CENTRAL. Use 
> + * mozilla::dom::EncodingUtils instead.

@deprecated

::: parser/html/nsHtml5StreamParser.cpp
@@ +322,2 @@
>    mCharset.Assign(aDecoderCharsetName);
> +  mUnicodeDecoder = EncodingUtils::DecoderForEncoding(mCharset);

aDecoderCharsetName?
Attachment #8337699 - Flags: review?(VYV03354) → review+
(In reply to Masatoshi Kimura [:emk] from comment #3)
> Comment on attachment 8337699 [details] [diff] [review]
> Add deCOMtaminated factories
> 
> Review of attachment 8337699 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: intl/uconv/idl/nsICharsetConverterManager.idl
> @@ +23,5 @@
> >  interface nsIUTF8StringEnumerator;
> >  
> >  /**
> > + * DON'T ADD NEW USES OF THIS INTERFACE TO MOZILLA-CENTRAL. Use 
> > + * mozilla::dom::EncodingUtils instead.
> 
> @deprecated

Added.

> ::: parser/html/nsHtml5StreamParser.cpp
> @@ +322,2 @@
> >    mCharset.Assign(aDecoderCharsetName);
> > +  mUnicodeDecoder = EncodingUtils::DecoderForEncoding(mCharset);
> 
> aDecoderCharsetName?

I think passing mCharset as the argument is more idiomatic. Maybe I should have changed the name of aDecoderCharsetName in a previous bug, though.

Thanks.

Landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/045001182507

Try run for sheriff reference:
https://tbpl.mozilla.org/?tree=Try&rev=4024f26b425b
https://hg.mozilla.org/mozilla-central/rev/045001182507
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
Blocks: 986548
You need to log in before you can comment on or make changes to this bug.