Closed
Bug 263462
Opened 20 years ago
Closed 18 years ago
NS_UnescapeURL doesn't honor esc_OnlyNonASCII
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jshin1987, Assigned: jshin1987)
References
Details
(Keywords: fixed-aviary1.0, intl)
Attachments
(1 file)
(deleted),
patch
|
darin.moz
:
review+
jst
:
superreview+
mkaply
:
approval1.7.5+
|
Details | Diff | Splinter Review |
NS_UnescapeURL doesn't check 'esc-OnlyNonASCII' flag. When unescaping, setting
it on should get ASCII characters to be skipped according to the 'documentation'
in nsEscape.h:
esc_OnlyNonASCII = PR_BIT(12), /* causes _graphic_ ascii octets (0x20-0x7E)
* to be skipped when escaping. causes all
* ascii octets to be skipped when unescaping *
I wrote that comment, but apparently I didn't implement it in nsEscape.cpp
leading to the discrepancy and bug 243504 (I specified 'esc_OnlyNonASCII' in
nsUTF8ConverterService.cpp expecting ASCII characters to be kept 'url-escaped'
when calling NS_UnescapeURL, but the flag has no effect and ASCII characters are
escaped).
I'm gonna upload a patch in a moment.
Assignee | ||
Comment 1•20 years ago
|
||
implement esc_OnlyNonASCII
Assignee | ||
Comment 2•20 years ago
|
||
Comment on attachment 161462 [details] [diff] [review]
patch
asking for r/sr
Attachment #161462 -
Flags: superreview?(jst)
Attachment #161462 -
Flags: review?(darin)
Comment 3•20 years ago
|
||
plussing for the aviary branch since this fix is required to fix 243504 which is
aviary1.0+
Flags: blocking-aviary1.0+
Comment 4•20 years ago
|
||
Comment on attachment 161462 [details] [diff] [review]
patch
sr=jst
Attachment #161462 -
Flags: superreview?(jst) → superreview+
Comment 5•20 years ago
|
||
Comment on attachment 161462 [details] [diff] [review]
patch
looks good, r=darin
Attachment #161462 -
Flags: review?(darin) → review+
Assignee | ||
Comment 7•20 years ago
|
||
Comment on attachment 161462 [details] [diff] [review]
patch
asking for a1.7
it's already gotten into aviary 1.0 branch
Attachment #161462 -
Flags: approval1.7.x?
Comment 8•20 years ago
|
||
Comment on attachment 161462 [details] [diff] [review]
patch
a=mkaply for 1.7
Attachment #161462 -
Flags: approval1.7.x? → approval1.7.x+
Assignee | ||
Comment 9•20 years ago
|
||
checked into 1.7 and trunk. thanks all.
Comment 10•20 years ago
|
||
I think this change to our escaping code caused:
Bug #273109
Comment 11•20 years ago
|
||
Re-open because patch for this bug is backed out on Thunderbird 1.0 release
build(temporary?) to resolve more serious regression, Bug 273109.
See Bug 273109 Comment #17.
This probably revived problem of Bug 243504(Bug 239192 also).
See Bug 243504 Comment #31.
Updated•20 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•20 years ago
|
Keywords: fixed-aviary1.0
Comment 12•20 years ago
|
||
clearing fixed 1.7.5 flag as the patch has been backed out of there as well
Keywords: fixed1.7.5
Updated•20 years ago
|
Flags: blocking1.8a6?
Comment 13•20 years ago
|
||
Doesn't look like this is going to make 1.8a6
Flags: blocking1.8b?
Flags: blocking1.8a6?
Flags: blocking1.8a6-
Assignee | ||
Comment 14•20 years ago
|
||
This was kept on trunk. This bug is only for 1.7/1.0 branches.
Version: Trunk → 1.7 Branch
Comment 15•20 years ago
|
||
I put this back on the thunderbird 1.0 branch. re-closing the bug.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 20 years ago
Keywords: fixed-aviary1.0
Resolution: --- → FIXED
Comment 16•20 years ago
|
||
Reopening to comfirm status on 1.7 branch.
Mscott, pacth looks to be still backed-out on 1.7 branch.
Is it intentional?
(Aviary)
> http://lxr.mozilla.org/aviarybranch/source/xpcom/io/nsEscape.cpp#437
> 437 PRBool ignoreNonAscii = (flags & esc_OnlyASCII);
> 438 PRBool ignoreAscii = (flags & esc_OnlyNonASCII);
> 439 PRBool writing = (flags & esc_AlwaysCopy);
(Mozilla 1.7 branch)
> http://lxr.mozilla.org/mozilla1.7/source/xpcom/io/nsEscape.cpp#437
> 437 PRBool ignoreNonAscii = (flags & esc_OnlyASCII);
> 438 PRBool writing = (flags & esc_AlwaysCopy);
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 17•20 years ago
|
||
(In reply to comment #16)
> Reopening to comfirm status on 1.7 branch.
>
> Mscott, pacth looks to be still backed-out on 1.7 branch.
> Is it intentional?
It cannot be relaned until I get approval for landing the patch for bug 274264
and land it in 1.7 branch.
Comment 18•20 years ago
|
||
Oh, delay by waiting for approval problem again...
If so, I think setting "blocking 1.7.6" flag is better.
Jshin, what do you think?
Updated•20 years ago
|
Flags: blocking1.7.6?
Comment 20•20 years ago
|
||
Is this fixed on the trunk? If so, should it be marked as such?
Flags: blocking1.8b? → blocking1.8b-
Comment 21•20 years ago
|
||
I made some updates to bug 243504 that I think contains some additional
information regarding the issue. Apparently it has been solved in the user
interface but not from the command line.
Updated•18 years ago
|
QA Contact: xpcom
Comment 22•18 years ago
|
||
The patch is apparently checked in a long time ago.
http://lxr.mozilla.org/mozilla/source/xpcom/io/nsEscape.cpp#384
Marking FIXED.
Status: REOPENED → RESOLVED
Closed: 20 years ago → 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•