Closed Bug 949666 Opened 6 years ago Closed 5 years ago

[e10s] Refactor file picker code to work better across processes

Categories

(Core :: DOM: Core & HTML, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
e10s m8+ ---

People

(Reporter: billm, Assigned: evilpie)

References

Details

Attachments

(1 file, 1 obsolete file)

I'm filing this bug to cover a few things:

1. The file picker dialog should return file handles, not just paths. That's the most reasonable way to get it working with sandboxing.

2. The code to determine the last directory where you used the file picker runs in the content process, but it makes much more sense for it to run in the parent.

3. The current proxied implementation of the file picker uses ::Show, which is model. We should switch to an API that uses ::Open.
I think we are done here.
Flags: needinfo?(wmccloskey)
Well, we still have problem #2. We could deal with that in some other way though. Maybe we just need to make the content pref service work in the child. Or we could move that code to the parent.
Flags: needinfo?(wmccloskey)
I think most stuff here is fixed, but problem #2 is still quite annoying, especially when using bugzilla. Renominating.
Blake implemented the content pref service a while ago, so we just need to remove the conditionals around that code. Want to take this Tom?
Attached patch Pass displayDirectory to parent (obsolete) — Splinter Review
Assignee: nobody → evilpies
Status: NEW → ASSIGNED
Attachment #8519443 - Flags: review?(wmccloskey)
Comment on attachment 8519443 [details] [diff] [review]
Pass displayDirectory to parent

Review of attachment 8519443 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great. Thanks!

::: dom/html/HTMLInputElement.cpp
@@ +708,2 @@
>        mInput->OwnerDoc(), lastUsedDir);
> +    printf("result: %u\n", uint32_t(rv));

Looks like you left this in by accident.
Attachment #8519443 - Flags: review?(wmccloskey) → review+
sorry had to back this out for test failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3721157&repo=mozilla-inbound (seems this was also in the try run but not sure how this was caused by this push)
Flags: needinfo?(evilpies)
I guess we should put back the content process check, but just for b2g.
Flags: needinfo?(evilpies)
Attached patch filepickerSplinter Review
I put back the ContentProcess check for b2g. Looks like this fixes the issue on try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ad1d0c7b82bb
Attachment #8519443 - Attachment is obsolete: true
Attachment #8521705 - Flags: review?(wmccloskey)
Attachment #8521705 - Flags: review?(wmccloskey) → review+
I made an other try push. It looks really clean, and not a single b2g mochitest failure https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b4cbcf12b411
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9e1814d648d
https://hg.mozilla.org/mozilla-central/rev/c9e1814d648d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Component: HTML: Form Submission → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.