Closed Bug 538433 Opened 14 years ago Closed 14 years ago

Incorporate Chromium's NSURL+Utils changes

Categories

(Camino Graveyard :: General, defect)

All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: alqahira, Assigned: alqahira)

References

()

Details

(Whiteboard: [camino-2.0.2])

Attachments

(1 file)

Chromium changed their copy of our NSURL+Utils to fix some deprecation warnings and also to fix some stuff that cl thought seemed dangerous.

[2:19pm] Wevah: we should probably do the stuff they did
[...]
[2:22pm] Wevah: oh yeah
[2:22pm] Wevah: our code still uses -stringWithCString
[2:22pm] Wevah: i should fix that
[2:22pm] Wevah: and +stringWithContentsOfFile
[2:23pm] Wevah: that's what they changed
[2:23pm] ardissone: i assume we can just take their patch straight, no?
[2:23pm] Wevah: we can
[2:23pm] Wevah: shouldn't cause any issues

(We should, however, check and see if MacIE .url files still work right; see pink's review comment.  I know I have more legacy .url files created by MacIE users than I have .url files obtained from Windows [or constructed myself from my .url<->.webloc scripts].)
AFAIK .url files from Mac IE use CRLF as their line terminator, so there's a good bet that they use a Windows charset. I still have Mac IE on my machine, so I'll test it.
This is the Chromium patch, less the lines where they commented-out our original code and indicated they modified it.

I ran this against my collection of original MacIE and self-generated .url files, and they all were parsed correctly.  I don't have any actual Windows .url files, but I have no reason to believe that they'd somehow be broken by a change designed to make the code more compatible with them :P

pink, should we add avi@chromium.org as a contributor to this file?  Not sure how that works, particularly when we're backporting a change that hasn't been explicitly upstreamed to us (why hasn't it? :P).
Assignee: nobody → alqahira
Status: NEW → ASSIGNED
Attachment #420913 - Flags: superreview?(mikepinkerton)
Comment on attachment 420913 [details] [diff] [review]
Chromium patch, ported

sr=pink.

No need to add Avi, I don't think. *shrug* I don't know how these things normally work.
Attachment #420913 - Flags: superreview?(mikepinkerton) → superreview+
Landed on cvs trunk.

I went ahead and landed this on the CAMINO_2_0_BRANCH, too, so we don't have to track down compilation failures if, for some awful reason, we're still using the branch when this breaks, and also because of the general concern with sketchiness of the old methods.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: camino2.0.2+
Resolution: --- → FIXED
Whiteboard: [camino-2.0.2]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: