Closed
Bug 271981
Opened 20 years ago
Closed 20 years ago
In <contentAreaUtils.js>, "Warning: redeclaration of var charset"
Categories
(SeaMonkey :: General, defect)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.8alpha6
People
(Reporter: sgautherie, Assigned: sgautherie)
References
Details
(Keywords: regression)
Attachments
(1 file, 3 obsolete files)
1.43 KB,
patch
|
Biesinger
:
review+
bzbarsky
:
superreview+
caillon
:
approval1.7.6-
|
Details | Diff | Splinter Review |
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a5) Gecko/20041122] (release) (W98SE) {{ Warning: redeclaration of var charset Source File: chrome://communicator/content/contentAreaUtils.js Line: 735, Column: 10 Source Code: var charset = getCharsetforSave(aDocument); }} Note: in MAS file only, not present in FireFox file.
Assignee | ||
Updated•20 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha6
Assignee | ||
Comment 1•20 years ago
|
||
Assignee: general → gautheri
Assignee | ||
Updated•20 years ago
|
Attachment #167182 -
Flags: superreview?(jst)
Attachment #167182 -
Flags: review?(cbiesinger)
Assignee | ||
Updated•20 years ago
|
Depends on: 263697
Keywords: regression
Comment 2•20 years ago
|
||
Comment on attachment 167182 [details] [diff] [review] (Av1) <contentAreaUtils.js> why not also move the getCharsetforSave call to the top of the function? I think I'd prefer neil to review this...
Attachment #167182 -
Flags: review?(cbiesinger) → review?(neil.parkwaycc.co.uk)
Assignee | ||
Comment 3•20 years ago
|
||
(In reply to comment #2) > (From update of attachment 167182 [details] [diff] [review]) > why not also move the getCharsetforSave call to the top of the function? Why not, indeed ! (I'm not the one who wants to decide. Neil !?)
Comment 4•20 years ago
|
||
cc'ing bz bceause he caused this ;) cc'ing neil so he sees the above question...
Comment 5•20 years ago
|
||
Why not just pass url.originCharset to unEscapeURIForUI?
Comment 6•20 years ago
|
||
In fact, we should never be trying to unescape it as UTF-8 (unless the origin character set was indeed UTF-8 of course), should we?
Comment 7•20 years ago
|
||
Updated•20 years ago
|
Attachment #167310 -
Flags: superreview?(bzbarsky)
Attachment #167310 -
Flags: review?(cbiesinger)
Comment 8•20 years ago
|
||
Comment on attachment 167310 [details] [diff] [review] (Bv1-MAS) As per my previous comments [Checked in: Comment 10] r=biesi. I'm not sure whether all callers do create the uri with a charset, but if they don't, that's a separate bug that should be fixed.
Attachment #167310 -
Flags: review?(cbiesinger) → review+
Comment 9•20 years ago
|
||
Comment on attachment 167310 [details] [diff] [review] (Bv1-MAS) As per my previous comments [Checked in: Comment 10] sr=bzbarsky, but please do check where that object comes from. I'm 99% sure that the caller messes up URI creation...
Attachment #167310 -
Flags: superreview?(bzbarsky) → superreview+
Comment 10•20 years ago
|
||
Fix checked in. The creator appears to be ioService.newURI via makeURI(aURL, aOriginCharset)
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Assignee | ||
Updated•20 years ago
|
Attachment #167182 -
Attachment is obsolete: true
Attachment #167182 -
Flags: superreview?(jst)
Attachment #167182 -
Flags: review?(neil.parkwaycc.co.uk)
Assignee | ||
Updated•20 years ago
|
Attachment #167310 -
Attachment description: As per my previous comments → As per my previous comments
[Checked in: Comment 10]
Attachment #167310 -
Attachment is obsolete: true
Assignee | ||
Comment 11•20 years ago
|
||
(In reply to comment #7) > Created an attachment (id=167310) > As per my previous comments Neil: Shouldn't your fix be ported to FireFox too ? http://lxr.mozilla.org/mozilla/source/browser/base/content/contentAreaUtils.js#733
Assignee | ||
Updated•19 years ago
|
Attachment #167310 -
Attachment description: As per my previous comments
[Checked in: Comment 10] → (Bv1-MAS) As per my previous comments
[Checked in: Comment 10]
Assignee | ||
Comment 12•19 years ago
|
||
Bv1 port to FireFox. I don't know the steps to test this one: Could you test/review/check in this patch ? Thanks.
Attachment #177661 -
Flags: review?(mconnor)
Assignee | ||
Updated•19 years ago
|
Attachment #167310 -
Flags: approval1.7.6?
Comment 13•19 years ago
|
||
Comment on attachment 167310 [details] [diff] [review] (Bv1-MAS) As per my previous comments [Checked in: Comment 10] Please stop asking for branch approval on strict warning patches. a-
Attachment #167310 -
Flags: approval1.7.6? → approval1.7.6-
Assignee | ||
Comment 14•19 years ago
|
||
(In reply to comment #13) > (From update of attachment 167310 [details] [diff] [review] [edit]) > Please stop asking for branch approval on strict warning patches. a- I was not complaining about the warning is the JS.Console (anymore): I though you would be interested in Neil's fix about the charset handling. But it seems I was wrong again :-(
Updated•19 years ago
|
Attachment #177661 -
Flags: review?(mconnor)
Assignee | ||
Comment 15•19 years ago
|
||
Cv1 updated to current Trunk. (Bv1 port to FireFox) I don't know the steps to test this one: Could you test/review/check in this patch ? Thanks.
Assignee | ||
Updated•19 years ago
|
Attachment #177661 -
Attachment is obsolete: true
Attachment #186642 -
Flags: review?(bryner)
Updated•19 years ago
|
Attachment #167310 -
Attachment is obsolete: false
Assignee | ||
Comment 16•19 years ago
|
||
(In reply to comment #15) > Created an attachment (id=186642) [edit] > (Cv1b-FF) <contentAreaUtils.js> You(/Toolkit guys) might want this for v1.8b3 !?
Comment 17•18 years ago
|
||
Comment on attachment 186642 [details] [diff] [review] (Cv1b-FF) <contentAreaUtils.js> [Checkin: See comment 21] I don't have a lot of time to spend on this and so I can't confidently say this does the right thing... can you ask someone else to review? It would help a lot if you could test your patches before requesting reviews, that's standard coding practice (reviewers aren't your testing service).
Attachment #186642 -
Flags: review?(bryner)
Assignee | ||
Comment 18•18 years ago
|
||
(In reply to comment #17) > (From update of attachment 186642 [details] [diff] [review] [edit]) > can you ask someone else to review? Would you have a name to suggest ? > It would help a lot if you could test your patches before requesting reviews, > that's standard coding practice (reviewers aren't your testing service). I know but, while I have the minimal skill to prepare this patch, I'm not a FireFox user and don't know how to test this fix. Yet I could follow steps if someone provided them. Helpwanted.
Keywords: helpwanted
Whiteboard: [SergeG: Need help to test Cv1b-FF patch.]
Assignee | ||
Comment 19•18 years ago
|
||
Comment on attachment 186642 [details] [diff] [review] (Cv1b-FF) <contentAreaUtils.js> [Checkin: See comment 21] Untested: see comment 18.
Attachment #186642 -
Flags: review?(bugs.mano)
Comment 20•18 years ago
|
||
Comment on attachment 186642 [details] [diff] [review] (Cv1b-FF) <contentAreaUtils.js> [Checkin: See comment 21] Get this tested first.
Attachment #186642 -
Flags: review?(bugs.mano)
Assignee | ||
Comment 21•17 years ago
|
||
Comment on attachment 186642 [details] [diff] [review] (Cv1b-FF) <contentAreaUtils.js> [Checkin: See comment 21] Actually, this was included in [ 1.71 mozilla.mano%sent.com 2005-06-28 07:41 Bug 294759 - port various contentAreaUtils fixes ('Save As', etc.) to the toolkit version. r=mconnor, a=bsmedberg. ] in the meantime... :-/
Attachment #186642 -
Attachment description: (Cv1b-FF) <contentAreaUtils.js> → (Cv1b-FF) <contentAreaUtils.js>
[Checkin: See comment 21]
Attachment #186642 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•