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

stylo: remove bogus optimization check in replace_rules() for visited styles. #17889

Merged
merged 1 commit into from
Jul 27, 2017

Conversation

chenpighead
Copy link
Contributor

@chenpighead chenpighead commented Jul 27, 2017

We skipped updating the rule nodes for visited rules during animation-only restyle.
However, this causes isseus that visited style overrides animation styles on visited element.
So, it turns out that we should update the visited rules even during animation-only restyle.

Gecko bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1381235


This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 27, 2017
@highfive
Copy link

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!

@chenpighead
Copy link
Contributor Author

@bors-servo r=hiro

@bors-servo
Copy link
Contributor

📌 Commit 372b2a1 has been approved by hiro

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 27, 2017
@chenpighead chenpighead changed the title stylo: fix the optimization check in replace_rules() for visited styles. stylo: remove bogus optimization check in replace_rules() for visited styles. Jul 27, 2017
… styles.

We skipped updating the rule nodes for visited rules during animation-only restyle.
However, this causes isseus that visited style overrides animation styles on visited element.
So, it turns out that we should update the visited rules even during animation-only restyle.

Gecko bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1381235
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 27, 2017
@chenpighead
Copy link
Contributor Author

@bors-servo r=hiro

@bors-servo
Copy link
Contributor

📌 Commit 000547b has been approved by hiro

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 27, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 000547b with merge d429561...

bors-servo pushed a commit that referenced this pull request Jul 27, 2017
stylo: remove bogus optimization check in replace_rules() for visited styles.

We skipped updating the rule nodes for visited rules during animation-only restyle.
However, this causes isseus that visited style overrides animation styles on visited element.
So, it turns out that we should update the visited rules even during animation-only restyle.

Gecko bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1381235

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

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: hiro
Pushing d429561 to master...

@bors-servo bors-servo merged commit 000547b into servo:master Jul 27, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants