Closed
Bug 1240700
Opened 5 years ago
Closed 5 years ago
"Save as PDF" requires Storage permission
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox46 fixed)
RESOLVED
FIXED
Firefox 46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: sebastian, Assigned: sebastian)
References
Details
Attachments
(1 file)
"Page -> Save as PDF" requires the Storage permission: Prompt if needed.
Assignee | ||
Comment 1•5 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/31409/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/31409/
Attachment #8709369 -
Flags: review?(nalexander)
Comment 2•5 years ago
|
||
Comment on attachment 8709369 [details] MozReview Request: Bug 1240700 - "Save as PDF": Request Storage permission. r?nalexander https://reviewboard.mozilla.org/r/31409/#review28143 lgtm. ::: mobile/android/chrome/content/browser.js:1425 (Diff revision 1) > + RuntimePermissions.waitForPermissions(RuntimePermissions.WRITE_EXTERNAL_STORAGE).then(function(permissionGranted) { I'm spit-balling here, but: there's something very unsatisfying about having to chain the result through. I know I was saying not to make `false` a failure, but consider adding a `errorIfNotPermitted` or something so that you can do: ``` RuntimePermissions.errorIfNotGranted(PERMISSION) .then(() => ...) .catch((e) => /* ignore */)) ``` In the code you have, people will forget the boolean check, possibly crashing the App on illegal access. In the code I have, we'll see lots of "failures" in the logcat, since folks will forget to catch-and-ignore. And worse, they'll catch-and-ignore real errors! Keep it how it is.
Attachment #8709369 -
Flags: review?(nalexander) → review+
Assignee | ||
Comment 3•5 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #2) > In the code you have, people will forget the boolean check, possibly > crashing the App on illegal access. And that's exactly what happened when I wrote this patch. :) I agree that we can and should make the API better. I'll move this to a follow-up bug and land this patch here to get the issue fixed in Nightly asap.
Assignee | ||
Comment 4•5 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #3) > I'll move this to a follow-up bug [..] bug 1240859
Assignee | ||
Comment 5•5 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/52e4a2480b5d96cfc6fb0da2a15735be72f33505 Bug 1240700 - "Save as PDF": Request Storage permission. r=nalexander
Comment 6•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/52e4a2480b5d
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Updated•2 months ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•