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

Fix two issues of parsing and serialization of mask shorthand #15298

Merged
merged 2 commits into from Feb 6, 2017

Conversation

@upsuper
Copy link
Member

upsuper commented Jan 30, 2017

This change is Reviewable

@highfive

This comment has been minimized.

Copy link

highfive commented Jan 30, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/shorthand/mask.mako.rs
  • @emilio: components/style/properties/shorthand/mask.mako.rs
@highfive

This comment has been minimized.

Copy link

highfive commented Jan 30, 2017

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!
@upsuper upsuper changed the title Don't require mask-mode to follow mask-image in mask shorthand Fix two issues of parsing and serialization of mask shorthand Jan 30, 2017
@nox

This comment has been minimized.

Copy link
Member

nox commented Jan 30, 2017

@bors-servo try

Seems like this needs tests.

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 30, 2017

⌛️ Trying commit 2f88bb6 with merge e6539bf...

bors-servo added a commit that referenced this pull request Jan 30, 2017
Fix two issues of parsing and serialization of mask shorthand

<!-- 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/15298)
<!-- Reviewable:end -->
@upsuper

This comment has been minimized.

Copy link
Member Author

upsuper commented Jan 30, 2017

Shorthand mask is currently Gecko-specific, and there are tests inside Gecko so I suppose we don't need further tests.

@upsuper

This comment has been minimized.

Copy link
Member Author

upsuper commented Jan 30, 2017

(Actually these issues are caught by Gecko's tests...)

@nox

This comment has been minimized.

Copy link
Member

nox commented Jan 30, 2017

I see. Will r+ once Homu is done with the try, I don't want to vex it.

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Jan 30, 2017

💔 Test failed - mac-dev-unit

@nox

This comment has been minimized.

Copy link
Member

nox commented Jan 30, 2017

The unit tests failed:

failures:

---- parsing::mask::mask_shorthand_should_not_parse_when_mode_specified_but_image_not stdout ----
	thread 'parsing::mask::mask_shorthand_should_not_parse_when_mode_specified_but_image_not' panicked at 'assertion failed: mask::parse_value(&context, &mut parser).is_err()', /Users/servo/buildbot/slave/mac-dev-unit/build/tests/unit/style/parsing/mask.rs:113
stack backtrace:
   1:        0x10a3d0d7c - std::sys::imp::backtrace::tracing::imp::write::h36dcd6719b30e145
   2:        0x10a3d496e - std::panicking::default_hook::{{closure}}::h4ef47a6ed549d2e6
   3:        0x10a3d4513 - std::panicking::default_hook::hd7b4c7a7e16abb43
   4:        0x10a3d4e27 - std::panicking::rust_panic_with_hook::h33761bada49f3713
   5:        0x109bb80f3 - std::panicking::begin_panic::h79533631a1075507
   6:        0x109d82954 - style_tests::parsing::mask::mask_shorthand_should_not_parse_when_mode_specified_but_image_not::h1f81835f40a474c0
   7:        0x10a32ae5e - <F as test::FnBox<T>>::call_box::hfce97b6a3bca902b
   8:        0x10a3d5d0a - __rust_maybe_catch_panic
   9:        0x10a31f098 - std::panicking::try::do_call::h4b6b5a01e5d0f1b4
  10:        0x10a3d5d0a - __rust_maybe_catch_panic
  11:        0x10a325dde - <F as alloc::boxed::FnBox<A>>::call_box::he619e4ac93c39e1b
  12:        0x10a3d4044 - std::sys::imp::thread::Thread::new::thread_start::he1b44499d44cc4a4
  13:     0x7fff8af9f059 - _pthread_body
  14:     0x7fff8af9efd6 - _pthread_start


failures:
    parsing::mask::mask_shorthand_should_not_parse_when_mode_specified_but_image_not
@upsuper

This comment has been minimized.

Copy link
Member Author

upsuper commented Feb 2, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 2, 2017

⌛️ Trying commit 1b8cba0 with merge 8548a30...

bors-servo added a commit that referenced this pull request Feb 2, 2017
Fix two issues of parsing and serialization of mask shorthand

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

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 2, 2017

💥 Test timed out

@upsuper

This comment has been minimized.

Copy link
Member Author

upsuper commented Feb 3, 2017

@bors-servo retry

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 3, 2017

⌛️ Trying commit 1b8cba0 with merge 106e6df...

bors-servo added a commit that referenced this pull request Feb 3, 2017
Fix two issues of parsing and serialization of mask shorthand

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

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 3, 2017

@nox

This comment has been minimized.

Copy link
Member

nox commented Feb 3, 2017

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 3, 2017

📌 Commit 1b8cba0 has been approved by nox

@upsuper upsuper force-pushed the upsuper:mask-shorthand branch from 1b8cba0 to 4a0482f Feb 5, 2017
@upsuper

This comment has been minimized.

Copy link
Member Author

upsuper commented Feb 5, 2017

@bors-servo r=nox

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 5, 2017

@upsuper: 🔑 Insufficient privileges: Not in reviewers

@KiChjang

This comment has been minimized.

Copy link
Member

KiChjang commented Feb 5, 2017

@bors-servo r=nox

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 5, 2017

📌 Commit 4a0482f has been approved by nox

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 5, 2017

⌛️ Testing commit 4a0482f with merge 5f0c294...

bors-servo added a commit that referenced this pull request Feb 5, 2017
Fix two issues of parsing and serialization of mask shorthand

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

This comment has been minimized.

Copy link
Contributor

bors-servo commented Feb 6, 2017

@bors-servo bors-servo merged commit 4a0482f into servo:master Feb 6, 2017
3 checks passed
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
@upsuper upsuper deleted the upsuper:mask-shorthand branch Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.