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

style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented. #18893

Merged
merged 4 commits into from Jan 5, 2018

Conversation

@emilio
Copy link
Member

emilio commented Oct 16, 2017

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.


This change is Reviewable

@highfive
Copy link

highfive commented Oct 16, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/gecko.mako.rs, components/style/style_adjuster.rs, components/style/dom.rs, components/style/gecko/wrapper.rs, components/style/properties/computed_value_flags.rs and 2 more
  • @canaltinova: components/style/properties/gecko.mako.rs, components/style/style_adjuster.rs, components/style/dom.rs, components/style/gecko/wrapper.rs, components/style/properties/computed_value_flags.rs and 2 more
  • @fitzgen: components/script/lib.rs, components/script/dom/node.rs, components/script_layout_interface/wrapper_traits.rs
  • @KiChjang: components/script/lib.rs, components/script/dom/node.rs, components/script_layout_interface/wrapper_traits.rs
@highfive
Copy link

highfive commented Oct 16, 2017

warning Warning warning

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

emilio commented Oct 16, 2017

@highfive highfive assigned SimonSapin and unassigned mbrubeck Oct 16, 2017
@emilio
Copy link
Member Author

emilio commented Oct 16, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Oct 16, 2017

Trying commit 871a016 with merge ec1164f...

bors-servo added a commit that referenced this pull request Oct 16, 2017
style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented.

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.
@emilio emilio force-pushed the emilio:bye-can-be-fragmented branch from 871a016 to 78a4194 Oct 16, 2017
@emilio
Copy link
Member Author

emilio commented Oct 16, 2017

@bors-servo try

  • Now passing tidy
@bors-servo
Copy link
Contributor

bors-servo commented Oct 16, 2017

Trying commit 78a4194 with merge dff6474...

bors-servo added a commit that referenced this pull request Oct 16, 2017
style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented.

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.

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

bors-servo commented Oct 16, 2017

💔 Test failed - linux-rel-css

@jdm
Copy link
Member

jdm commented Oct 16, 2017

  ▶ FAIL [expected PASS] /css-flexbox-1_dev/html/flexbox_columns.htm
  └   → /css-flexbox-1_dev/html/flexbox_columns.htm e038cebd05342d3dd4d4ddc05038107516f916c4
/css-flexbox-1_dev/html/reference/flexbox_columns-ref.htm b162d7afebb1dac86e4f86aae00e6a06654fd405
Testing e038cebd05342d3dd4d4ddc05038107516f916c4 == b162d7afebb1dac86e4f86aae00e6a06654fd405
@emilio
Copy link
Member Author

emilio commented Oct 16, 2017

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Oct 16, 2017

Trying commit 78a4194 with merge 2bef97c...

bors-servo added a commit that referenced this pull request Oct 16, 2017
style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented.

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.

<!-- 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/18893)
<!-- Reviewable:end -->
@SimonSapin
Copy link
Member

SimonSapin commented Oct 16, 2017

@bors-servo r+


Reviewed 14 of 14 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@bors-servo
Copy link
Contributor

bors-servo commented Oct 16, 2017

📌 Commit 78a4194 has been approved by SimonSapin

@emilio
Copy link
Member Author

emilio commented Oct 16, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Oct 16, 2017

Testing commit 78a4194 with merge a90280d...

bors-servo added a commit that referenced this pull request Oct 16, 2017
style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented.

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.

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

bors-servo commented Oct 16, 2017

💔 Test failed - mac-rel-css1

@emilio emilio force-pushed the emilio:bye-can-be-fragmented branch from 20918aa to 3bbd80d Jan 4, 2018
@highfive highfive removed the S-tests-failed label Jan 4, 2018
@emilio
Copy link
Member Author

emilio commented Jan 4, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jan 4, 2018

Trying commit 3bbd80d with merge 1dfc856...

bors-servo added a commit that referenced this pull request Jan 4, 2018
style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented.

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.

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

bors-servo commented Jan 4, 2018

💔 Test failed - mac-rel-wpt2

@emilio
Copy link
Member Author

emilio commented Jan 4, 2018

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 4, 2018

Trying commit 3bbd80d with merge 30586fd...

bors-servo added a commit that referenced this pull request Jan 4, 2018
style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented.

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.

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

bors-servo commented Jan 4, 2018

💔 Test failed - mac-rel-wpt4

@jdm
Copy link
Member

jdm commented Jan 4, 2018

  ▶ FAIL [expected PASS] /css/css-flexbox/flexbox_columns.html
  └   → /css/css-flexbox/flexbox_columns.html e038cebd05342d3dd4d4ddc05038107516f916c4
/css/css-flexbox/flexbox_columns-ref.html b162d7afebb1dac86e4f86aae00e6a06654fd405
Testing e038cebd05342d3dd4d4ddc05038107516f916c4 == b162d7afebb1dac86e4f86aae00e6a06654fd405

This consistently fails.

@emilio
Copy link
Member Author

emilio commented Jan 4, 2018

@SimonSapin I can't repro that test failure, and I'm pretty confident that if it was passing before it was just by chance. In any case, this is the right thing to do, as of right now Servo doesn't propagate the can_be_fragmented bit in any sort of right way, and this patch fixes it.

Mind if I land it with that test marked as failing?

@SimonSapin
Copy link
Member

SimonSapin commented Jan 5, 2018

It indeed sounds likely that flexbox+multicol doesn’t actually work well together today. Making as failing sounds fine.

If it was passing before this patch, it was just by chance, and I can't repro
the failure.
@highfive highfive removed the S-tests-failed label Jan 5, 2018
@emilio
Copy link
Member Author

emilio commented Jan 5, 2018

@bors-servo r=SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2018

📌 Commit 4ee9822 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2018

Testing commit 4ee9822 with merge 83a8891...

bors-servo added a commit that referenced this pull request Jan 5, 2018
style: Remove TNode::set_can_be_fragmented and TNode::can_be_fragmented.

Replace them instead by a computed value flag, the same way as the
IS_IN_DISPLAY_NONE_SUBTREE flag works.

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

bors-servo commented Jan 5, 2018

@bors-servo bors-servo merged commit 4ee9822 into servo:master Jan 5, 2018
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@emilio emilio deleted the emilio:bye-can-be-fragmented branch Jan 5, 2018
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.