Skip to content
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

Bug1367283 #17075

Merged
merged 8 commits into from May 29, 2017

Conversation

Projects
None yet
5 participants
@dadaa
Copy link
Contributor

commented May 29, 2017

This PR is to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1367283



This change is Reviewable

@highfive

This comment has been minimized.

Copy link

commented May 29, 2017

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @wafflespeanut (or someone else) soon.

@highfive

This comment has been minimized.

Copy link

commented May 29, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/longhand/inherited_text.mako.rs, components/style/properties/gecko.mako.rs, components/style/properties/longhand/list.mako.rs, components/style/properties/longhand/position.mako.rs, components/style/properties/longhand/background.mako.rs and 16 more
  • @emilio: components/style/properties/longhand/inherited_text.mako.rs, components/style/properties/gecko.mako.rs, components/style/properties/longhand/list.mako.rs, components/style/properties/longhand/position.mako.rs, components/style/properties/longhand/background.mako.rs and 16 more
@highfive

This comment has been minimized.

Copy link

commented May 29, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify style code, but no tests are modified. Please consider adding a test!
@dadaa

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2017

These patches have been reviewed by @hiikezoe

@wafflespeanut

This comment has been minimized.

Copy link
Member

commented May 29, 2017

@bors-servo r=hiikezoe

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

📌 Commit 4a36b08 has been approved by hiikezoe

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

⌛️ Testing commit 4a36b08 with merge e1d3dc9...

bors-servo added a commit that referenced this pull request May 29, 2017

Auto merge of #17075 - dadaa:bug1367283, r=hiikezoe
Bug1367283

<!-- Please describe your changes on the following line: -->
This PR is to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1367283

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes. The tests will land in dom/animation/test of m-c.  Test code is patch 9 of https://bugzilla.mozilla.org/show_bug.cgi?id=1367283

<!-- 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/17075)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

💔 Test failed - android

@emilio

This comment has been minimized.

Copy link
Member

commented May 29, 2017

This seems to be failing a unit test (not sure if legitimately or not):

---- parsing::ui::test_moz_user_select stdout ----
	thread 'parsing::ui::test_moz_user_select' panicked at 'Failed to parse -moz-none: ()', /checkout/src/libcore/result.rs:859
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:355
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:365
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:549
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:511
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:495
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:471
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:69
   9: core::result::unwrap_failed
             at /checkout/src/libcore/macros.rs:29
  10: <core::result::Result<T, E>>::expect
             at /checkout/src/libcore/result.rs:761
  11: style_tests::parsing::ui::test_moz_user_select::{{closure}}
             at ./parsing/mod.rs:67
  12: style_tests::parsing::parse
             at ./parsing/mod.rs:26
  13: style_tests::parsing::ui::test_moz_user_select
             at ./parsing/ui.rs:23
  14: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1440
             at /checkout/src/libcore/ops.rs:2626
             at /checkout/src/libtest/lib.rs:140
  15: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98

You can run them with ./mach test-unit -p style.

@dadaa dadaa force-pushed the dadaa:bug1367283 branch from 4a36b08 to 8a5c7a5 May 29, 2017

@dadaa

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2017

Thanks @emilio !
I fixed the fail.

@dadaa

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2017

@bors-servo r=hiikezoe

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

@dadaa: 🔑 Insufficient privileges: Not in reviewers

@emilio

This comment has been minimized.

Copy link
Member

commented May 29, 2017

@bors-servo r=hiikezoe delegate+

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

✌️ @dadaa can now approve this pull request

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

📌 Commit 8a5c7a5 has been approved by hiikezoe

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

⌛️ Testing commit 8a5c7a5 with merge 63533ce...

bors-servo added a commit that referenced this pull request May 29, 2017

Auto merge of #17075 - dadaa:bug1367283, r=hiikezoe
Bug1367283

<!-- Please describe your changes on the following line: -->
This PR is to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1367283

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes. The tests will land in dom/animation/test of m-c.  Test code is patch 9 of https://bugzilla.mozilla.org/show_bug.cgi?id=1367283

<!-- 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/17075)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented May 29, 2017

@bors-servo bors-servo merged commit 8a5c7a5 into servo:master May 29, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.