| Summary: | Vertical writing mode can overflow fixed size grandparent container | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Victor Carbune <vcarbune> | ||||||||||||||||||
| Component: | CSS | Assignee: | Ojan Vafai <ojan> | ||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||
| Severity: | Normal | CC: | dglazkov, eric, gustavo, hyatt, jchaffraix, mitz, ojan, tony, webkit.review.bot, xan.lopez | ||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||
| OS: | All | ||||||||||||||||||||
| Bug Depends on: | 88116 | ||||||||||||||||||||
| Bug Blocks: | 62048 | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
Created attachment 144402 [details]
screenshot of how it renders
This test case is very complicated. Just to be clear, the only bug here is that verticalGrowingLeftContainer grows outside of verticalCueContainer, right? Or are there multiple bugs? Created attachment 145115 [details]
reduced test case
Created attachment 145134 [details]
Patch
Hyatt, this should be a simple 2-line review. Mind taking a quick look? I don't think there's anyone but you and I familiar with vertical writing mode. :) (In reply to comment #2) > This test case is very complicated. Just to be clear, the only bug here is that verticalGrowingLeftContainer grows outside of verticalCueContainer, right? Or are there multiple bugs? Right, that should be it. Sorry about the fairly complicated csae, but that's what I was working on. (In reply to comment #6) > (In reply to comment #2) > > This test case is very complicated. Just to be clear, the only bug here is that verticalGrowingLeftContainer grows outside of verticalCueContainer, right? Or are there multiple bugs? > > Right, that should be it. Sorry about the fairly complicated csae, but that's what I was working on. No worries. Just wanted to make sure I wasn't missing something. Comment on attachment 145134 [details] Patch Attachment 145134 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12862446 New failing tests: perf/object-keys.html fast/html/details-writing-mode.html Created attachment 145191 [details]
Archive of layout-test-results from ec2-cr-linux-01
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 145134 [details]
Patch
The fix looks OK, but it seems to make us regress a table test. It's a shot in the dark but RenderTableSection::calcRowLogicalHeight has some special case for overrideHeight that may be impacted.
Comment on attachment 145134 [details]
Patch
Yeah, I saw that. I should have removed the r?. I was working on it last night. It turns out to be tricky to fix. Different uses of overrideHeight seem to include or not incude border+padding. Making the same causes more tests to fail for reasons I don't understand. :(
Created attachment 145398 [details]
Patch
Comment on attachment 145398 [details] Patch This patch depends on the one in bug 88116. Comment on attachment 145398 [details] Patch Attachment 145398 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12863678 Comment on attachment 145398 [details] Patch Attachment 145398 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12872581 Comment on attachment 145398 [details] Patch Attachment 145398 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12869636 Comment on attachment 145398 [details] Patch Attachment 145398 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12868664 Comment on attachment 145398 [details] Patch Attachment 145398 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12860823 Comment on attachment 145398 [details] Patch Attachment 145398 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12888105 Comment on attachment 145398 [details] Patch Attachment 145398 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12888139 Created attachment 145848 [details]
Patch
Comment on attachment 145848 [details]
Patch
r=me assuming the tests are passing.
Comment on attachment 145848 [details] Patch Clearing flags on attachment: 145848 Committed r119562: <http://trac.webkit.org/changeset/119562> All reviewed patches have been landed. Closing bug. The initial html page attached still isn't rendered at all properly, but I guess there are other related bugs filed? Created attachment 146615 [details]
reduction
Here's a new reduction. This doesn't seem to be specific to flexbox, but with vertical writing mode.
A few comments:
- If you remove #middle, the text no longer overflows.
- If you keep #middle and set the height to 300px, the text no longer overflows.
- If you keep #middle and set the height to 100%, the text overflow.
It seems like setting the height of #middle to 100% should work.
Focusing the reduction. This bug has too much noise with the flexbox patch in it. Moving over to bug 93655 to focus on the remaining writing-mode issue. |
Created attachment 144401 [details] html page The height of the parent containing block is not respected and the text overflows. (-webkit-flex: 1) There are multiple nested layouts in the example (using display:-webkit-flex)