Implement overflow:clip #35103
Implement overflow:clip #35103
Conversation
longvatrong111
requested review from
mrobinson,
Loirooriol and
nicoburns
as code owners
Signed-off-by: batu_hoang <[email protected]>
longvatrong111
force-pushed
the
overflow-clip
branch
from
ab7692e to
709226a
Compare
| style_dom = { git = "https://github.com/servo/stylo", package = "dom", branch = "2025-01-02" } | ||
| style_traits = { git = "https://github.com/servo/stylo", branch = "2025-01-02", features = ["servo"] } | ||
| style_malloc_size_of = { package = "malloc_size_of", git = "https://github.com/servo/stylo", branch = "2025-01-02", features = ["servo"] } | ||
| style = { git = "https://github.com/servo/stylo", rev = "f54c445dc4ccd1ce20016636dcb85981a82354eb", features = ["servo"] } |
There was a problem hiding this comment.
why revert?
if you need Clip enum, maybe add feature gecko, instead of revert.
There was a problem hiding this comment.
rev is not reverting anything, if that's what you mean. It's a git revision: https://git-scm.com/docs/gitrevisions
| style_dom = { git = "https://github.com/servo/stylo", package = "dom", branch = "2025-01-02" } | ||
| style_traits = { git = "https://github.com/servo/stylo", branch = "2025-01-02", features = ["servo"] } | ||
| style_malloc_size_of = { package = "malloc_size_of", git = "https://github.com/servo/stylo", branch = "2025-01-02", features = ["servo"] } | ||
| style = { git = "https://github.com/servo/stylo", rev = "f54c445dc4ccd1ce20016636dcb85981a82354eb", features = ["servo"] } |
There was a problem hiding this comment.
rev is not reverting anything, if that's what you mean. It's a git revision: https://git-scm.com/docs/gitrevisions
| _ if (style.get_box().overflow_x != StyleOverflow::Visible && | ||
| style.get_box().overflow_x != StyleOverflow::Clip) || | ||
| (style.get_box().overflow_y != StyleOverflow::Visible && | ||
| style.get_box().overflow_y != StyleOverflow::Clip) || |
There was a problem hiding this comment.
This is legacy layout, so I wouldn't bother with overflow: clip here.
But if you are changing it, then at least use is_scrollable():
| _ if (style.get_box().overflow_x != StyleOverflow::Visible && | |
| style.get_box().overflow_x != StyleOverflow::Clip) || | |
| (style.get_box().overflow_y != StyleOverflow::Visible && | |
| style.get_box().overflow_y != StyleOverflow::Clip) || | |
| _ if style.get_box().overflow_x.is_scrollable() || | |
| style.get_box().overflow_y.is_scrollable() || |
Or in fact both axes must have the same scrollability, so just checking one should suffice.
| if self.style.effective_overflow().x != ComputedOverflow::Clip && | ||
| self.style.effective_overflow().y != ComputedOverflow::Clip |
There was a problem hiding this comment.
Please store self.style.effective_overflow() in a variable.
| let clip_margin = self.style.get_margin().overflow_clip_margin; | ||
| clip_rect.size.width += Au::from(clip_margin); | ||
| clip_rect.size.height += Au::from(clip_margin); |
There was a problem hiding this comment.
| let clip_margin = self.style.get_margin().overflow_clip_margin; | |
| clip_rect.size.width += Au::from(clip_margin); | |
| clip_rect.size.height += Au::from(clip_margin); | |
| let clip_margin = Au::from(self.style.get_margin().overflow_clip_margin); | |
| clip_rect.size.width += clip_margin; | |
| clip_rect.size.height += clip_margin; |
| @@ -190,6 +191,39 @@ impl BoxFragment { | |||
| ) | |||
| } | |||
|
|
|||
| pub fn clip_overflow(&self, containing_block_rect: &PhysicalRect<Au>) -> PhysicalRect<Au> { | |||
| // Area that the box is allowed to paint in. | |||
| let mut clip_rect = match self.style.get_box()._servo_overflow_clip_box { | |||
There was a problem hiding this comment.
If you are enabling overflow-clip-box-inline/block then I would check these here.
Otherwise don't enable them.
| @@ -1,2 +1,2 @@ | |||
| [clip-001.html] | |||
| expected: FAIL | |||
| expected: PASS | |||
There was a problem hiding this comment.
You can remove this ini file entirely if all subtests pass.
Ditto below.
Is it possible to get a quick rundown on which test cases fail and why? I don't think it is critical to cover every test case, but it would be good to have an understand of what is causing the rest of the tests to fail. |
This PR implements
overflow:clipoverflow:clipoverflow-clip-marginandoverflow-clip-boxin Stylo, the parser works welloverflow-clip-margincompounded with other CSS properties still fail.Stylo PR: servo/stylo#111
Test cases:
servo/tests/wpt/tests/css/css-overflow/
cc: @xiaochengh
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors