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

Remove unnecessary ClipDisplayItems in box_.rs #1578

Merged
merged 2 commits into from
Feb 4, 2014

Conversation

pradeep90
Copy link
Contributor

For ScannedTextBox and ImageBox, the ClipDisplayItem's child_list is
currently always empty.

ClipDisplayItem is used to implement overflow hidden and should only be
created for block containers, as per
http://www.w3.org/TR/CSS2/visufx.html#propdef-overflow

@hoppipolla-critic-bot
Copy link

Critic review: https://critic.hoppipolla.co.uk/r/665

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@highfive
Copy link

warning Warning warning

  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!
  • @pradeep90, please confirm that src/test/html/acid1.html and your favourite wikipedia page still render correctly!

@pradeep90
Copy link
Contributor Author

Right now, it is failing for some edge case ('fail to find parent item').

I will fix it and get back here.

@pradeep90
Copy link
Contributor Author

Fixed the bug where an img with display: block (no children) was failing to find the parent item for the empty children display_list.

+ Annotate functions with CSS Reference sections.
+ Clean up comments and whitespace.
@pradeep90
Copy link
Contributor Author

Rebased on latest Servo code.

r? @larsbergstrom

For ScannedTextBox and ImageBox, the ClipDisplayItem's child_list is
currently always empty.

ClipDisplayItem is used to implement overflow hidden and should only be
created for block containers, as per
http://www.w3.org/TR/CSS2/visufx.html#propdef-overflow

Take care of the case when a BlockFlow has no children display items -
like an ImageBox with "display: block".
@pradeep90
Copy link
Contributor Author

@larsbergstrom The FIXME is not needed any more because that part is contained within if self.is_block_container().

I think the only case when InlineFlow will have children flows is when inline-blocks are supported. Then, they will have a GenericBox and thus will generate a ClipDisplayItemClass in which the children display items will go.

@pradeep90
Copy link
Contributor Author

@larsbergstrom I don't think that was considered as an r+ for the PR. Nothing has happened :)

larsbergstrom referenced this pull request in ksh8281/servo Feb 4, 2014
bors-servo pushed a commit that referenced this pull request Feb 4, 2014
…om,larsbergstrom

For ScannedTextBox and ImageBox, the ClipDisplayItem's child_list is
currently always empty.

ClipDisplayItem is used to implement overflow hidden and should only be
created for block containers, as per
http://www.w3.org/TR/CSS2/visufx.html#propdef-overflow
@bors-servo bors-servo merged commit 35a869d into servo:master Feb 4, 2014
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.

None yet

5 participants