Bug 68497

Summary: incorrect height with height:auto and writing-mode:vertical-rl
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: Layout and RenderingAssignee: Ojan Vafai <ojan>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, hyatt, tabatkins, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch for landing
none
Patch for landing webkit.review.bot: commit-queue-

Ojan Vafai
Reported 2011-09-20 18:59:23 PDT
I'm not 100% sure about this, but as per my reading of http://dev.w3.org/csswg/css3-writing-modes/#orthogonal-flows, we do the wrong thing with the following case: <div style="height:300px; -webkit-writing-mode: horizontal-tb"> <div style="height: auto; -webkit-writing-mode: vertical-rl; border: 1px solid">asdf</div> </div> The physical height of the inner div should be shrink-wrapped to it's content, instead it's 300px. http://plexode.com/eval3/#ht=%3Cdiv%20style%3D%22height%3A300px%3B%20-webkit-writing-mode%3A%20horizontal-tb%22%3E%0A%20%20%20%20%3Cdiv%20style%3D%22height%3A%20auto%3B%20-webkit-writing-mode%3A%20vertical-rl%3B%20border%3A%201px%20solid%22%3Easdf%3C%2Fdiv%3E%0A%3C%2Fdiv%3E%0A&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1 Am I misreading the spec? Is the spec stupid?
Attachments
Patch (9.53 KB, patch)
2011-09-22 14:23 PDT, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
Patch for landing (1023.57 KB, patch)
2011-10-14 14:51 PDT, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
Patch for landing (60.02 KB, patch)
2011-10-14 15:03 PDT, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
Patch for landing (60.01 KB, patch)
2011-10-15 14:47 PDT, Ojan Vafai 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 2 2011-09-22 14:23:11 PDT
Created attachment 108402 [details] Patch
Ojan Vafai
Comment 3 2011-09-22 14:25:14 PDT
Here's a patch that implements the shrink-wrapping using the containing block size and falling back to the initial containing block size. I'll mark it for review once the writing modes spec changes to require this behavior.
Ojan Vafai
Comment 4 2011-09-22 14:27:56 PDT
Comment on attachment 108402 [details] Patch On second thought, this patch brings us closer to the current spec in that we shrink-wrap height:auto. Should the spec text wrt containing block not change, we can always change the shrink-wrapping to always use the initial containing block.
WebKit Review Bot
Comment 5 2011-09-23 03:10:28 PDT
Comment on attachment 108402 [details] Patch Attachment 108402 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9815143 New failing tests: fast/table/028-vertical.html fast/table/height-percent-test-vertical.html
Dave Hyatt
Comment 6 2011-10-13 14:26:41 PDT
Comment on attachment 108402 [details] Patch r=me, make sure you get all tests though.
Ojan Vafai
Comment 7 2011-10-14 14:51:38 PDT
Created attachment 111085 [details] Patch for landing
Ojan Vafai
Comment 8 2011-10-14 15:01:02 PDT
Comment on attachment 111085 [details] Patch for landing Woah. Land-safely did something crazy with the ChangeLog entry. :(
Ojan Vafai
Comment 9 2011-10-14 15:03:04 PDT
Created attachment 111086 [details] Patch for landing
WebKit Review Bot
Comment 10 2011-10-15 00:19:51 PDT
Comment on attachment 111086 [details] Patch for landing Rejecting attachment 111086 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 NOBODY (OOPS!) found in /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog does not appear to be a valid reviewer according to committers.py. ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/10073279
Ojan Vafai
Comment 11 2011-10-15 14:47:30 PDT
Created attachment 111147 [details] Patch for landing
WebKit Review Bot
Comment 12 2011-10-15 15:51:52 PDT
Comment on attachment 111147 [details] Patch for landing Rejecting attachment 111147 [details] from commit-queue. New failing tests: fast/table/028-vertical.html fast/table/height-percent-test-vertical.html Full output: http://queues.webkit.org/results/10080178
Ojan Vafai
Comment 13 2011-10-17 14:49:15 PDT
Committed r97654: <http://trac.webkit.org/changeset/97654>
Ojan Vafai
Comment 14 2011-10-17 15:55:34 PDT
Reverted r97654 for reason: Caused a number of Chromium failures. Committed r97662: <http://trac.webkit.org/changeset/97662>
Ojan Vafai
Comment 15 2011-10-17 20:12:11 PDT
Committed r97707: <http://trac.webkit.org/changeset/97707>
Note You need to log in before you can comment on or make changes to this bug.