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 restyling on viewport resize. #12838

Merged
merged 4 commits into from Aug 16, 2016
Merged

Fix restyling on viewport resize. #12838

merged 4 commits into from Aug 16, 2016

Conversation

@emilio
Copy link
Member

emilio commented Aug 12, 2016

Fixes #12835


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes OR

This change is Reviewable

@highfive
Copy link

highfive commented Aug 12, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/lib.rs, components/style/custom_properties.rs, components/style/traversal.rs, components/style/stylesheets.rs, components/style/selector_matching.rs, components/style/properties/properties.mako.rs, components/style/properties/helpers.mako.rs, components/style/values/specified/mod.rs, components/style/cascade_info.rs, components/style/animation.rs, components/style/matching.rs
@highfive
Copy link

highfive commented Aug 12, 2016

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@emilio
Copy link
Member Author

emilio commented Aug 12, 2016

@highfive highfive assigned SimonSapin and unassigned metajack Aug 12, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Aug 14, 2016

The latest upstream changes (presumably #12839) made this pull request unmergeable. Please resolve the merge conflicts.

emilio added 2 commits Aug 12, 2016
@emilio emilio force-pushed the emilio:viewport branch from a7baf95 to b00bf27 Aug 14, 2016
@emilio
Copy link
Member Author

emilio commented Aug 14, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Aug 14, 2016

Trying commit b00bf27 with merge 711c4f0...

bors-servo added a commit that referenced this pull request Aug 14, 2016
Fix restyling on viewport resize.

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

Fixes #12835

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes OR

<!-- 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/12838)
<!-- Reviewable:end -->
@emilio emilio force-pushed the emilio:viewport branch from b00bf27 to c3bcf4d Aug 14, 2016
@emilio
Copy link
Member Author

emilio commented Aug 14, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Aug 14, 2016

Trying commit c3bcf4d with merge 559b3e9...

bors-servo added a commit that referenced this pull request Aug 14, 2016
Fix restyling on viewport resize.

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

Fixes #12835

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes OR

<!-- 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/12838)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 14, 2016

💔 Test failed - linux-rel

@emilio emilio force-pushed the emilio:viewport branch from c3bcf4d to 4ab6eaa Aug 14, 2016
@emilio
Copy link
Member Author

emilio commented Aug 14, 2016

@bors-servo: try

Apparently the syntax:

if cfg!(whatever) {
    self.field_only_existing_if_cfg_whatever = foo;
}

broke with the rustup I'm stupid.

@bors-servo
Copy link
Contributor

bors-servo commented Aug 14, 2016

Trying commit 4ab6eaa with merge 56c790b...

bors-servo added a commit that referenced this pull request Aug 14, 2016
Fix restyling on viewport resize.

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

Fixes #12835

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes OR

<!-- 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/12838)
<!-- Reviewable:end -->
@eddyb
Copy link
Contributor

eddyb commented Aug 14, 2016

@emilio That never worked, if false doesn't make typeck ignore the field access.

@emilio
Copy link
Member Author

emilio commented Aug 14, 2016

FTR, yeah, @eddyb is right, and I'm stupid for misremembering having built this on release locally before the rustup.

@emilio emilio removed the S-needs-rebase label Aug 15, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Aug 15, 2016

💥 Test timed out

@SimonSapin
Copy link
Member

SimonSapin commented Aug 16, 2016

r=me with s/red/green/ if you’ve seen the test fail


Reviewed 1 of 1 files at r1, 2 of 2 files at r2, 12 of 12 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


tests/wpt/mozilla/tests/css/dirty_viewport.html, line 7 [r3] (raw file):

<iframe seamless src="dirty_viewport_inner.html" width="500" height="500"></iframe>
<script>
  window.onload = function() {

Did this test fail without the code changes?


tests/wpt/mozilla/tests/css/dirty_viewport_ref.html, line 4 [r3] (raw file):

<style>
  div {
    background: red;

Nit: red is generally used to indicate a test failure. (On success, it the red is hidden/behind something else). Please change this to green.


Comments from Reviewable

emilio added 2 commits Aug 12, 2016
…ade, and use it for viewport units.
…te, since now it's useless.
@emilio emilio force-pushed the emilio:viewport branch from 4ab6eaa to fc12841 Aug 16, 2016
@emilio
Copy link
Member Author

emilio commented Aug 16, 2016

@bors-servo: r=SimonSapin


Review status: 13 of 16 files reviewed at latest revision, 2 unresolved discussions.


tests/wpt/mozilla/tests/css/dirty_viewport.html, line 7 [r3] (raw file):

Previously, SimonSapin (Simon Sapin) wrote…

Did this test fail without the code changes?

Yep, that's the first thing I tested, but just in case I double-checked :P

tests/wpt/mozilla/tests/css/dirty_viewport_ref.html, line 4 [r3] (raw file):

Previously, SimonSapin (Simon Sapin) wrote…

Nit: red is generally used to indicate a test failure. (On success, it the red is hidden/behind something else). Please change this to green.

Done.

Comments from Reviewable

@bors-servo
Copy link
Contributor

bors-servo commented Aug 16, 2016

📌 Commit fc12841 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Aug 16, 2016

Testing commit fc12841 with merge 49431be...

bors-servo added a commit that referenced this pull request Aug 16, 2016
Fix restyling on viewport resize.

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

Fixes #12835

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes OR

<!-- 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/12838)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 16, 2016

@bors-servo bors-servo merged commit fc12841 into servo:master Aug 16, 2016
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
@emilio emilio deleted the emilio:viewport branch Aug 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.