Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabled tinyfiledialogs when running headless #15967

Merged
merged 1 commit into from Mar 16, 2017
Merged

Disabled tinyfiledialogs when running headless #15967

merged 1 commit into from Mar 16, 2017

Conversation

@asajeffrey
Copy link
Member

asajeffrey commented Mar 15, 2017 •
edited by larsbergstrom

Disable tinyfiledialogs when running in headless mode.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #15965 and #15962.
  • These changes do not require tests because we don't test tinyfiledialog pop-ups.

This change is Reviewable
@highfive
Copy link

highfive commented Mar 15, 2017

Heads up! This PR modifies the following files:

  • @fitzgen: components/script/dom/window.rs, components/script/dom/permissions.rs
  • @KiChjang: components/net/filemanager_thread.rs, components/script/dom/window.rs, components/script/dom/permissions.rs
@highfive
Copy link

highfive commented Mar 15, 2017

warning Warning warning

  • These commits modify net and script code, but no tests are modified. Please consider adding a test!
@asajeffrey
Copy link
Member Author

asajeffrey commented Mar 15, 2017

cc @jdm
@jdm
jdm reviewed Mar 15, 2017
View changes
components/net/filemanager_thread.rs Outdated
@@ -557,7 +558,8 @@ impl FileManagerStore {

fn select_files_pref_enabled() -> bool {
PREFS.get("dom.testing.htmlinputelement.select_files.enabled")
.as_boolean().unwrap_or(false)
.as_boolean().unwrap_or(false) &&
!opts::get().headless

This comment has been minimized.

Sign in to view
@jdm

jdm Mar 15, 2017

Member

Why is this change necessary? Wont' this make us use TFD when we're in headless mode, since the callers invoke the UI when it returns false?

This comment has been minimized.

Sign in to view
@asajeffrey

asajeffrey Mar 15, 2017

Author Member

Oops, got the semantics of this flag inverted.
@asajeffrey
Copy link
Member Author

asajeffrey commented Mar 15, 2017

The unit tests were failing because the UI provider wasn't being called in headless mode. The fix is to move the headless check inside the UI provider.
@jdm
Copy link
Member

jdm commented Mar 16, 2017

Looks good to merge after squashing.
@asajeffrey
Disabled tinyfiledialogs when running headless.
Loading status checks…
f05577c
@asajeffrey asajeffrey force-pushed the asajeffrey:headless-disable-tinyfd branch from f714b22 to f05577c Mar 16, 2017
@asajeffrey
Copy link
Member Author

asajeffrey commented Mar 16, 2017

Squashed. @bors-servo r=jdm
@bors-servo
Copy link
Contributor

bors-servo commented Mar 16, 2017

📌 Commit f05577c has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Mar 16, 2017

⌛ Testing commit f05577c with merge e34aac0...
bors-servo added a commit that referenced this pull request Mar 16, 2017
@bors-servo
Auto merge of #15967 - asajeffrey:headless-disable-tinyfd, r=jdm
Loading status checks…
e34aac0 
Disabled tinyfiledialogs when running headless

<!-- Please describe your changes on the following line: -->

Disable tinyfiledialogs when running in headless mode.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15965 and #15962.
- [X] These changes do not require tests because we don't test tinyfiledialog pop-ups.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15967)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Mar 16, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
Approved by: jdm
Pushing e34aac0 to master...
@bors-servo bors-servo merged commit f05577c into servo:master Mar 16, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@asajeffrey asajeffrey mentioned this pull request Mar 16, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm

Assignees

@jdm jdm

Labels
I-intermittent I-papercut
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Disable tinyfiledialog popups when running in headless mode
4 participants
@asajeffrey @highfive @jdm @bors-servo
You can’t perform that action at this time.