Closed Bug 1428258 Opened 6 years ago Closed 6 years ago

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

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla60

Tracking Flags:

Tracking

Status

firefox59

---

wontfix

firefox60

---

fixed

People

(Reporter: emk, Assigned: emk)

References

Details

(Keywords: addon-compat)

Attachments

(6 files)

Bug 1428258 - Stop using GetNativePath in chrome/. 6 years ago Masatoshi Kimura [:emk] 59 bytes, text/x-review-board-request	mayhemer : review+	Details
Bug 1428258 - Stop using GetNativePath in url-classifier. 6 years ago Masatoshi Kimura [:emk] 59 bytes, text/x-review-board-request	mossop : review+	Details
Bug 1428258 - Stop using GetNativePath in nsAppRunner.cpp. 6 years ago Masatoshi Kimura [:emk] 59 bytes, text/x-review-board-request	mossop : review+	Details
Bug 1428258 - Stop using GetNativePath in gfx/. 6 years ago Masatoshi Kimura [:emk] 59 bytes, text/x-review-board-request	jrmuizel : review+	Details
Bug 1428258 - Stop using GetNativePath in Telemetry.cpp. 6 years ago Masatoshi Kimura [:emk] 59 bytes, text/x-review-board-request	Dexter : review+	Details
Bug 1428258 - Disallow nsIFile::GetNativePath on Windows. 6 years ago Masatoshi Kimura [:emk] 59 bytes, text/x-review-board-request	froydnj : review+	Details

Masatoshi Kimura [:emk]

Assignee

Description

•

6 years ago

+++ 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 hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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/

Dave Townsend [:mossop]

Comment 4

•

6 years ago

mozreview-review

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

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

Attachment #8949025 - Flags: review?(dtownsend) → review+

Dave Townsend [:mossop]

Comment 5

•

6 years ago

mozreview-review

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

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

Attachment #8949026 - Flags: review?(dtownsend) → review+

Honza Bambas (:mayhemer)

Comment 6

•

6 years ago

mozreview-review

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

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

Attachment #8949024 - Flags: review?(honzab.moz) → review+

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Georg Fritzsche [:gfritzsche]

Updated

•

6 years ago

Assignee: nobody → VYV03354

Masatoshi Kimura [:emk]

Assignee

Updated

•

6 years ago

Status: NEW → ASSIGNED

Jeff Muizelaar [:jrmuizel]

Comment 12

•

6 years ago

mozreview-review

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

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

Attachment #8950897 - Flags: review?(jmuizelaar) → review+

Masatoshi Kimura [:emk]

Assignee

Updated

•

6 years ago

Depends on: 1428568

Comment hidden (mozreview-request)

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

Comment hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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 hidden (mozreview-request)

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/

Nathan Froyd [:froydnj]

Comment 19

•

6 years ago

mozreview-review

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

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

Thank you!

Attachment #8952183 - Flags: review?(nfroyd) → review+

Comment hidden (mozreview-request)

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/

Alessio Placitelli [:Dexter]

Comment 21

•

6 years ago

mozreview-review

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!

Attachment #8950898 - Flags: review+

Alessio Placitelli [:Dexter]

Updated

•

6 years ago

Attachment #8950898 - Flags: review?(gfritzsche)

Pulsebot

Comment 22

•

6 years ago

Pushed by VYV03354@nifty.ne.jp:
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

Dorel Luca [:dluca]

Comment 23

•

6 years ago

bugherder

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

Status: ASSIGNED → RESOLVED

Closed: 6 years ago

status-firefox60: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla60

Honza Bambas (:mayhemer)

Comment 24

•

6 years ago

Ah, THIS ONE breaks comm-central build.

Flags: needinfo?(VYV03354)

Masatoshi Kimura [:emk]

Assignee

Comment 25

•

6 years ago

See bug 1440278.

Depends on: 1440278

Flags: needinfo?(VYV03354)

Julien Cristau [:jcristau]

Updated

•

6 years ago

status-firefox59: affected → wontfix

Ionuț Goldan [:igoldan]

Updated

•

6 years ago

Depends on: 1441112

Jan Beich

Updated

•

6 years ago

Depends on: 1442746

You need to log in before you can comment on or make changes to this bug.