Add filepicker #11717

izgzhen · 2016-06-10T19:19:16Z

Add file picker based on tinyfiledialog to the file manager implementation.

Changes:

Add the picker invocation code
Rewrite unit test to accommodate the change
Patch up htmlinputelement to make things work

Related to #11131.

This change is

highfive · 2016-06-10T19:19:19Z

Heads up! This PR modifies the following files:

@KiChjang: components/net/filemanager_thread.rs, components/script/dom/htmlinputelement.rs, components/net_traits/filemanager_thread.rs, components/net_traits/filemanager_thread.rs

highfive · 2016-06-10T19:19:20Z

Warning

These commits modify net and script code, but no tests are modified. Please consider adding a test!

izgzhen · 2016-06-10T19:22:54Z

Still work in progress, I have one doubt about how to refactor the original unit test of filemanager_thread (tests/unit/net/filemanager_thread.rs): Since we are sending a SelectFile to the file manager, it will cause the interactive dialog to pop up and this might not be testable for a CI.

izgzhen · 2016-06-11T08:41:11Z

Forget to mention: Another problem is we might need an inverse of mime_guess, which when given mime types, can give us back a list of possible file extensions. relevant spec: https://html.spec.whatwg.org/multipage/forms.html#attr-input-accept.

jdm · 2016-06-11T10:30:05Z

Make it testable the same way we test HTTP authorization dialogs: use a trait in the code that invokes the UI, and have tests provide an implementation that doesn't use the actual dialog.

Manishearth · 2016-06-11T10:31:09Z

Yeah, that's a better solution.

highfive · 2016-06-12T06:03:50Z

New code was committed to pull request.

izgzhen · 2016-06-12T06:05:47Z

The test-unit is fixed. Still have filter pattern to fill.

highfive · 2016-06-12T08:27:43Z

New code was committed to pull request.

Manishearth · 2016-06-14T07:49:13Z

components/script/dom/htmlinputelement.rs

@@ -1228,3 +1230,8 @@ impl Activatable for HTMLInputElement {
        }
    }
 }
+
+fn filter_from_accept(_s: DOMString) -> Vec<FilterPattern> {
+    unimplemented!()


should we just return an empty vector with a todo so that this doesn't crash?

@Manishearth I contacted upstream, and the functionality necessary for this is implemented, so I am waiting for him to upload the new version actually 😅

Nice!

So that this can merge, though, could we still use the empty vec (till that gets implemented) so that it doesn't crash and can be used?

good, I will fix it another day

Manishearth · 2016-06-14T07:50:20Z

r=me with that unimplemented replaced with an empty vec and a todo

@bors-servo delegate+

bors-servo · 2016-06-14T07:50:21Z

✌️ @izgzhen can now approve this pull request

highfive · 2016-06-14T10:30:38Z

New code was committed to pull request.

izgzhen · 2016-06-14T10:32:00Z

@bors-servo r=Manishearth

bors-servo · 2016-06-14T10:32:00Z

📌 Commit 0406f1b has been approved by Manishearth

bors-servo · 2016-06-14T10:32:04Z

⌛ Testing commit 0406f1b with merge 386b52b...


        Auto merge of #11717 - izgzhen:filepicker, r=Manishearth

Add filepicker Add file picker based on tinyfiledialog to the file manager implementation. Changes: - [x] Add the picker invocation code - [x] Rewrite unit test to accommodate the change - [x] Patch up `htmlinputelement` to make things work  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes are related to #11131.  - [x] There are tests for these changes OR   --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11717)

bors-servo · 2016-06-14T10:44:14Z

💔 Test failed - windows

izgzhen · 2016-06-14T10:47:31Z

Ahh nice Windows..... = = I should have thought that

bors-servo · 2016-06-14T12:15:57Z

💔 Test failed - windows

Manishearth · 2016-06-14T12:17:10Z

C:\buildbot\slave\windows\build\components\net\filemanager_thread.rs:17:5: 17:20 error: unresolved import `tinyfiledialogs`. There is no `tinyfiledialogs` in the crate root [E0432]
C:\buildbot\slave\windows\build\components\net\filemanager_thread.rs:17 use tinyfiledialogs;


        filepicker

highfive · 2016-06-14T12:23:06Z

New code was committed to pull request.

Manishearth · 2016-06-14T12:27:45Z

@bors-servo r+

bors-servo · 2016-06-14T12:27:46Z

📌 Commit 256c7e8 has been approved by Manishearth

bors-servo · 2016-06-14T12:27:48Z

⌛ Testing commit 256c7e8 with merge 0cfae3a...


        Auto merge of #11717 - izgzhen:filepicker, r=Manishearth

Add filepicker Add file picker based on tinyfiledialog to the file manager implementation. Changes: - [x] Add the picker invocation code - [x] Rewrite unit test to accommodate the change - [x] Patch up `htmlinputelement` to make things work  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes are related to #11131.  - [x] There are tests for these changes OR   --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11717)

bors-servo · 2016-06-14T13:23:39Z

💔 Test failed - linux-rel

highfive · 2016-06-14T13:23:43Z

  ▶ CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/change_parentage.html
  │ 
  │ Xlib:  extension &#34;XFree86-VidModeExtension&#34; missing on display &#34;:0&#34;.
  │ ERROR:constellation::constellation: Panic: ScriptThread: received an event message for a layout channel that is not associated with this script thread.This is a bug.
  │ ERROR:constellation::constellation: Backtrace:
  │ frame #0  - 0x00007f86b87b824d - backtrace::backtrace::trace::hccde8df28b4db2a2
  │ frame #1  - 0x00007f86b87b81d5 - backtrace::capture::Backtrace::new::h42f95930bb8c5ee8
  │ frame #2  - 0x00007f86b77df219 - _&lt;F as alloc..boxed..FnBox&lt;A&gt;&gt;::call_box::h89adae1a802be550
  │ frame #3  - 0x00007f86b87a9a88 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::he2b22674ad1748f3
  │ frame #4  - 0x00007f86b8a7766c - std::panicking::rust_panic_with_hook::h983af77c1a2e581b
  │ frame #5  - 0x00007f86b8a91a71 - std::panicking::begin_panic::he426e15a3766089a
  │ frame #6  - 0x00007f86b8a78eda - std::panicking::begin_panic_fmt::hdddb415186c241e7
  │ frame #7  - 0x00007f86b8a91a0e - rust_begin_unwind
  │ frame #8  - 0x00007f86b8ac800f - core::panicking::panic_fmt::hf4e16cb7f0d41a25
  │ frame #9  - 0x00007f86b8acf6c4 - core::option::expect_failed::hdb92832549f56a85
  │ frame #10 - 0x00007f86b77ec251 - script::script_thread::ScriptThread::handle_msg_from_script::h47e979ae1e7fb676
  │ frame #11 - 0x00007f86b783806f - script::script_thread::ScriptThread::handle_msgs::_$u7b$$u7b$closure$u7d$$u7d$::hb50cf7fd97943b65
  │ frame #12 - 0x00007f86b781f607 - script::script_thread::ScriptThread::handle_msgs::h1e1abab71191c950
  │ frame #13 - 0x00007f86b77dd907 - std::panicking::try::call::h78dcd5319adf08fa
  │ frame #14 - 0x00007f86b8a9bc6b - __rust_try
  │ frame #15 - 0x00007f86b8a9bc0e - __rust_maybe_catch_panic
  │ frame #16 - 0x00007f86b77deb7d - _&lt;F as alloc..boxed..FnBox&lt;A&gt;&gt;::call_box::hce043844655ea6a9
  │ frame #17 - 0x00007f86b8a8fc34 - std::sys::thread::Thread::new::thread_start::h9c883b6d445ece46
  │ frame #18 - 0x00007f86b48cf183 - start_thread
  │ frame #19 - 0x00007f86b43e637c - clone
  │ frame #20 - 0x0000000000000000 - &lt;unknown&gt;
  │ 
  └ ERROR:constellation::constellation: Pipeline failed in hard-fail mode.  Crashing!

jdm · 2016-06-14T13:26:06Z

@bors-servo: retry

#11693

bors-servo · 2016-06-14T13:26:08Z

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel, mac-dev-unit, mac-rel-css...

bors-servo · 2016-06-14T14:13:19Z

💔 Test failed - linux-rel

highfive · 2016-06-14T14:13:31Z

  ▶ CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/change_parentage.html
  │ 
  │ Xlib:  extension &#34;XFree86-VidModeExtension&#34; missing on display &#34;:0&#34;.
  │ ERROR:constellation::constellation: Panic: ScriptThread: received an event message for a layout channel that is not associated with this script thread.This is a bug.
  │ ERROR:constellation::constellation: Backtrace:
  │ frame #0  - 0x00007f90f9f258ad - backtrace::backtrace::trace::hccde8df28b4db2a2
  │ frame #1  - 0x00007f90f9f25835 - backtrace::capture::Backtrace::new::h42f95930bb8c5ee8
  │ frame #2  - 0x00007f90f8f4c899 - _&lt;F as alloc..boxed..FnBox&lt;A&gt;&gt;::call_box::h89adae1a802be550
  │ frame #3  - 0x00007f90f9f170e8 - util::panicking::initiate_panic_hook::_$u7b$$u7b$closure$u7d$$u7d$::_$u7b$$u7b$closure$u7d$$u7d$::he2b22674ad1748f3
  │ frame #4  - 0x00007f90fa1e4ccc - std::panicking::rust_panic_with_hook::h983af77c1a2e581b
  │ frame #5  - 0x00007f90fa1ff0d1 - std::panicking::begin_panic::he426e15a3766089a
  │ frame #6  - 0x00007f90fa1e653a - std::panicking::begin_panic_fmt::hdddb415186c241e7
  │ frame #7  - 0x00007f90fa1ff06e - rust_begin_unwind
  │ frame #8  - 0x00007f90fa23566f - core::panicking::panic_fmt::hf4e16cb7f0d41a25
  │ frame #9  - 0x00007f90fa23cd24 - core::option::expect_failed::hdb92832549f56a85
  │ frame #10 - 0x00007f90f8f598d1 - script::script_thread::ScriptThread::handle_msg_from_script::h47e979ae1e7fb676
  │ frame #11 - 0x00007f90f8fa56ef - script::script_thread::ScriptThread::handle_msgs::_$u7b$$u7b$closure$u7d$$u7d$::hb50cf7fd97943b65
  │ frame #12 - 0x00007f90f8f8cc87 - script::script_thread::ScriptThread::handle_msgs::h1e1abab71191c950
  │ frame #13 - 0x00007f90f8f4af87 - std::panicking::try::call::h78dcd5319adf08fa
  │ frame #14 - 0x00007f90fa2092cb - __rust_try
  │ frame #15 - 0x00007f90fa20926e - __rust_maybe_catch_panic
  │ frame #16 - 0x00007f90f8f4c1fd - _&lt;F as alloc..boxed..FnBox&lt;A&gt;&gt;::call_box::hce043844655ea6a9
  │ frame #17 - 0x00007f90fa1fd294 - std::sys::thread::Thread::new::thread_start::h9c883b6d445ece46
  │ frame #18 - 0x00007f90f603c181 - start_thread
  │ frame #19 - 0x00007f90f5b5347c - __clone
  │ frame #20 - 0x0000000000000000 - &lt;unknown&gt;
  │ 
  └ ERROR:constellation::constellation: Pipeline failed in hard-fail mode.  Crashing!

izgzhen · 2016-06-14T14:34:27Z

@bors-servo: retry

#11693

bors-servo · 2016-06-14T14:34:29Z

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel...

bors-servo · 2016-06-14T15:21:57Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

highfive assigned wafflespeanut Jun 10, 2016

highfive added the S-awaiting-review label Jun 10, 2016

Manishearth self-assigned this Jun 11, 2016

izgzhen force-pushed the izgzhen:filepicker branch from bc4ddb0 to 5b51050 Jun 12, 2016

izgzhen force-pushed the izgzhen:filepicker branch from 5b51050 to 81a6573 Jun 12, 2016

Manishearth reviewed Jun 14, 2016
View changes

izgzhen force-pushed the izgzhen:filepicker branch from 81a6573 to 0406f1b Jun 14, 2016

highfive unassigned wafflespeanut Jun 14, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Jun 14, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 14, 2016

filepicker

Loading status checks…

256c7e8

izgzhen force-pushed the izgzhen:filepicker branch from 4e0e2f7 to 256c7e8 Jun 14, 2016

highfive removed the S-tests-failed label Jun 14, 2016

highfive added the S-awaiting-review label Jun 14, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Jun 14, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 14, 2016

highfive added the S-tests-failed label Jun 14, 2016

bors-servo merged commit 256c7e8 into servo:master Jun 14, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

This was referenced Jun 14, 2016

Implement private browsing for mozbrowser #11544

Merged

Implement Blob URL's DOM interfaces #11716

Merged

servo / servo

Add filepicker #11717

Add filepicker #11717

izgzhen commented Jun 10, 2016 •

edited

highfive commented Jun 10, 2016

highfive commented Jun 10, 2016

izgzhen commented Jun 10, 2016

izgzhen commented Jun 11, 2016 •

edited

jdm commented Jun 11, 2016

Manishearth commented Jun 11, 2016

highfive commented Jun 12, 2016

izgzhen commented Jun 12, 2016 •

edited

highfive commented Jun 12, 2016

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Manishearth commented Jun 14, 2016 •

edited

bors-servo commented Jun 14, 2016

highfive commented Jun 14, 2016

izgzhen commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

izgzhen commented Jun 14, 2016

bors-servo commented Jun 14, 2016

Manishearth commented Jun 14, 2016

highfive commented Jun 14, 2016

Manishearth commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

highfive commented Jun 14, 2016

jdm commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

highfive commented Jun 14, 2016

izgzhen commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

servo / servo

Join GitHub today

Add filepicker #11717

Add filepicker #11717

Conversation

izgzhen commented Jun 10, 2016 • edited

highfive commented Jun 10, 2016

highfive commented Jun 10, 2016

izgzhen commented Jun 10, 2016

izgzhen commented Jun 11, 2016 • edited

jdm commented Jun 11, 2016

Manishearth commented Jun 11, 2016

highfive commented Jun 12, 2016

izgzhen commented Jun 12, 2016 • edited

highfive commented Jun 12, 2016

This comment has been minimized.

Manishearth Jun 14, 2016

This comment has been minimized.

izgzhen Jun 14, 2016

This comment has been minimized.

Manishearth Jun 14, 2016 • edited

This comment has been minimized.

izgzhen Jun 14, 2016

Manishearth commented Jun 14, 2016 • edited

bors-servo commented Jun 14, 2016

highfive commented Jun 14, 2016

izgzhen commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

izgzhen commented Jun 14, 2016

bors-servo commented Jun 14, 2016

Manishearth commented Jun 14, 2016

highfive commented Jun 14, 2016

Manishearth commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

highfive commented Jun 14, 2016

jdm commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

highfive commented Jun 14, 2016

izgzhen commented Jun 14, 2016

bors-servo commented Jun 14, 2016

bors-servo commented Jun 14, 2016

izgzhen commented Jun 10, 2016 •

edited

izgzhen commented Jun 11, 2016 •

edited

izgzhen commented Jun 12, 2016 •

edited

Manishearth Jun 14, 2016 •

edited

Manishearth commented Jun 14, 2016 •

edited