1428258 - Move nsIFile::GetNativePath to a different interface (or remove it entirely)

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Masatoshi Kimura [:emk]]

+++ This bug was initially created as a clone of Bug #685236 +++

nsIFile::GetNativePath is a footgun, because it returns a char* and that breaks on Unicode paths.

We should move it to a new interface (nsIFileTotallyUnsafeYouShouldntUseIt) or remove it entirely if it's possible.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Bug 1428258 - Stop using GetNativePath in chrome/.

Review commit: https://reviewboard.mozilla.org/r/218422/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218422/

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: Bug 1428258 - Stop using GetNativePath in url-classifier.

Review commit: https://reviewboard.mozilla.org/r/218424/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218424/

Comment 3

[Masatoshi Kimura [:emk]]

Attachment: Bug 1428258 - Stop using GetNativePath in nsAppRunner.cpp.

GetPersistentDescriptor is suitable for serialization purpose.

Review commit: https://reviewboard.mozilla.org/r/218426/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/218426/

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8949025 [details]
Bug 1428258 - Stop using GetNativePath in url-classifier.

https://reviewboard.mozilla.org/r/218424/#review224352

Comment 5

[Dave Townsend [:mossop]]

Comment on attachment 8949026 [details]
Bug 1428258 - Stop using GetNativePath in nsAppRunner.cpp.

https://reviewboard.mozilla.org/r/218426/#review224354

Comment 6

[Honza Bambas (:mayhemer)]

Comment on attachment 8949024 [details]
Bug 1428258 - Stop using GetNativePath in chrome/.

https://reviewboard.mozilla.org/r/218422/#review224890

Comment 7

[Masatoshi Kimura [:emk]]

Attachment: Bug 1428258 - Stop using GetNativePath in gfx/.

Review commit: https://reviewboard.mozilla.org/r/220152/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220152/

Comment 8

[Masatoshi Kimura [:emk]]

Attachment: Bug 1428258 - Stop using GetNativePath in Telemetry.cpp.

Review commit: https://reviewboard.mozilla.org/r/220154/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220154/

Comment 9

[Masatoshi Kimura [:emk]]

Comment on attachment 8949024 [details]
Bug 1428258 - Stop using GetNativePath in chrome/.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218422/diff/1-2/

Comment 10

[Masatoshi Kimura [:emk]]

Comment on attachment 8949025 [details]
Bug 1428258 - Stop using GetNativePath in url-classifier.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218424/diff/1-2/

Comment 11

[Masatoshi Kimura [:emk]]

Comment on attachment 8949026 [details]
Bug 1428258 - Stop using GetNativePath in nsAppRunner.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218426/diff/1-2/

Comment 12

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8950897 [details]
Bug 1428258 - Stop using GetNativePath in gfx/.

https://reviewboard.mozilla.org/r/220152/#review226046

Comment 13

[Masatoshi Kimura [:emk]]

Attachment: Bug 1428258 - Disallow nsIFile::GetNativePath on Windows.

Review commit: https://reviewboard.mozilla.org/r/221426/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/221426/

Comment 14

[Masatoshi Kimura [:emk]]

Comment on attachment 8949024 [details]
Bug 1428258 - Stop using GetNativePath in chrome/.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218422/diff/2-3/

Comment 15

[Masatoshi Kimura [:emk]]

Comment on attachment 8949025 [details]
Bug 1428258 - Stop using GetNativePath in url-classifier.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218424/diff/2-3/

Comment 16

[Masatoshi Kimura [:emk]]

Comment on attachment 8949026 [details]
Bug 1428258 - Stop using GetNativePath in nsAppRunner.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/218426/diff/2-3/

Comment 17

[Masatoshi Kimura [:emk]]

Comment on attachment 8950897 [details]
Bug 1428258 - Stop using GetNativePath in gfx/.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220152/diff/1-2/

Comment 18

[Masatoshi Kimura [:emk]]

Comment on attachment 8950898 [details]
Bug 1428258 - Stop using GetNativePath in Telemetry.cpp.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/220154/diff/1-2/

Comment 19

[Nathan Froyd [:froydnj]]

Comment on attachment 8952183 [details]
Bug 1428258 - Disallow nsIFile::GetNativePath on Windows.

https://reviewboard.mozilla.org/r/221426/#review227236

Thank you!

Comment 20

[Masatoshi Kimura [:emk]]

Comment on attachment 8952183 [details]
Bug 1428258 - Disallow nsIFile::GetNativePath on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/221426/diff/1-2/

Comment 21

[Alessio Placitelli [:Dexter]]

Comment on attachment 8950898 [details]
Bug 1428258 - Stop using GetNativePath in Telemetry.cpp.

https://reviewboard.mozilla.org/r/220154/#review227880

Stealing the review from :gfritzsche before :) This looks good to me!

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ffa5068067a7
Stop using GetNativePath in chrome/. r=mayhemer
https://hg.mozilla.org/integration/autoland/rev/c87bbb86f843
Stop using GetNativePath in url-classifier. r=mossop
https://hg.mozilla.org/integration/autoland/rev/3089af2474ef
Stop using GetNativePath in nsAppRunner.cpp. r=mossop
https://hg.mozilla.org/integration/autoland/rev/e3b034c3ac97
Stop using GetNativePath in gfx/. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/e28a37787c93
Stop using GetNativePath in Telemetry.cpp. r=Dexter
https://hg.mozilla.org/integration/autoland/rev/ee1367eae850
Disallow nsIFile::GetNativePath on Windows. r=froydnj

Comment 23

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ffa5068067a7
https://hg.mozilla.org/mozilla-central/rev/c87bbb86f843
https://hg.mozilla.org/mozilla-central/rev/3089af2474ef
https://hg.mozilla.org/mozilla-central/rev/e3b034c3ac97
https://hg.mozilla.org/mozilla-central/rev/e28a37787c93
https://hg.mozilla.org/mozilla-central/rev/ee1367eae850

Comment 24

[Honza Bambas (:mayhemer)]

Ah, THIS ONE breaks comm-central build.

Comment 25

[Masatoshi Kimura [:emk]]

See bug 1440278.