nsIProtocolHandler should document that newURI inherits charset from aBaseURI

RESOLVED INVALID

Status

()

P5
normal
RESOLVED INVALID
15 years ago
a year ago

People

(Reporter: Biesinger, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-would-take][good first bug])

Attachments

(1 attachment, 1 obsolete attachment)

1.07 KB, patch
Details | Diff | Splinter Review
darin said in bug 24867 comment 172:
>> +  var targetURI = IOS.newURI(fileURL.file.leafName, null, targetBaseURI);
>> 
>> you don't need a charset here? it seems you can use
>> contentFrame.document.characterSet...
>
>No need.  The charset is inherited from targetBaseURI.

nsIIOService refers to nsIProtocolHandler for details on newURI, and
nsIProtocolHandler nowhere describes that the charset should be inherited from
the base uri:
http://lxr.mozilla.org/seamonkey/source/netwerk/base/public/nsIProtocolHandler.idl#68

Updated

13 years ago
Assignee: darin → nobody
QA Contact: benc → networking
Status: NEW → UNCONFIRMED
Ever confirmed: false
Whiteboard: [necko-would-take][good first bug]

Comment 1

3 years ago
I'd be happy to take this one!

Comment 2

3 years ago
Can i take this bug?

Comment 3

3 years ago
Hi
I am a newbie and would like to work on this bug. can somebody assign me to it and help me in the process?
It doesn't need to be assigned - feel free to work on it and upload a patch. :mayhemer would be a good reviewer for the patch.

Comment 5

3 years ago
Since there is no language tag to it . Can somebody specify the tag as well as tell me where I can find the bits of code to work on

Comment 6

3 years ago
Since there is no language tag to it . Can somebody specify the tag as well as tell me where I can find the bits of code to work on
comment 0 is just referring to the idl files of the same name

Comment 8

2 years ago
Is this still a bug? I fetched the code from mozilla-central, and in the nsIProtocolHandler.idl file it says
>> /* if aOriginCharset is null, then UTF-8 encoding is assumed */
>> newURI(... aSpec,
>>        ... aOriginCharset,
>>        ... aBaseUri)

(I shortened most of the documentation)

Comment 9

2 years ago
(In reply to han from comment #8)
> Is this still a bug? I fetched the code from mozilla-central, and in the
> nsIProtocolHandler.idl file it says
> >> /* if aOriginCharset is null, then UTF-8 encoding is assumed */
> >> newURI(... aSpec,
> >>        ... aOriginCharset,
> >>        ... aBaseUri)
> 
> (I shortened most of the documentation)

I think this is counter to the original statement in the bug, which is that the character set would be inherited. This seems to state that, rather than being inherited, UTF-8 is assumed if null is passed. 
A link to the updated idl is https://hg.mozilla.org/mozilla-central/file/tip/netwerk/base/nsIProtocolHandler.idl#l68 . Can this be closed?
That is talking about something else... the documentation still does not mention that aBaseURI affects the charset of the resulting URI.

Comment 11

a year ago
Created attachment 8904037 [details]
Git patch file for Bug 240523

Updated

a year ago
Attachment #8904037 - Attachment is obsolete: true
Attachment #8904037 - Attachment is patch: false

Comment 12

a year ago
Created attachment 8904039 [details] [diff] [review]
HG patch for bug 240523

Comment 14

a year ago
This bug has a patch attached to it for the past 2 months. And the patch seems to provide the correct fix. Why is this ticket still open?
Flags: needinfo?(honzab.moz)

Comment 15

a year ago
zy, could you hint what is further pending on this ticket? Thanks
Flags: needinfo?(zy)

Updated

a year ago
Flags: needinfo?(zy)
Attachment #8904039 - Flags: review?(honzab.moz)
After bug 1322874, we do not inherit charset from aBaseURI anymore. We do not even have charset information in URI objects now.
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
Resolution: --- → INVALID
Attachment #8904039 - Flags: review?(honzab.moz)
see comment 16.
Flags: needinfo?(honzab.moz)
You need to log in before you can comment on or make changes to this bug.