Bug 68497 - incorrect height with height:auto and writing-mode:vertical-rl
Summary: incorrect height with height:auto and writing-mode:vertical-rl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-20 18:59 PDT by Ojan Vafai
Modified: 2011-10-17 20:12 PDT (History)
5 users (show)

See Also:


Attachments
Patch (9.53 KB, patch)
2011-09-22 14:23 PDT, Ojan Vafai
no flags Details | Formatted Diff | Diff
Patch for landing (1023.57 KB, patch)
2011-10-14 14:51 PDT, Ojan Vafai
no flags Details | Formatted Diff | Diff
Patch for landing (60.02 KB, patch)
2011-10-14 15:03 PDT, Ojan Vafai
no flags Details | Formatted Diff | Diff
Patch for landing (60.01 KB, patch)
2011-10-15 14:47 PDT, Ojan Vafai
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 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?
Comment 2 Ojan Vafai 2011-09-22 14:23:11 PDT
Created attachment 108402 [details]
Patch
Comment 3 Ojan Vafai 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.
Comment 4 Ojan Vafai 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.
Comment 5 WebKit Review Bot 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
Comment 6 Dave Hyatt 2011-10-13 14:26:41 PDT
Comment on attachment 108402 [details]
Patch

r=me, make sure you get all tests though.
Comment 7 Ojan Vafai 2011-10-14 14:51:38 PDT
Created attachment 111085 [details]
Patch for landing
Comment 8 Ojan Vafai 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. :(
Comment 9 Ojan Vafai 2011-10-14 15:03:04 PDT
Created attachment 111086 [details]
Patch for landing
Comment 10 WebKit Review Bot 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
Comment 11 Ojan Vafai 2011-10-15 14:47:30 PDT
Created attachment 111147 [details]
Patch for landing
Comment 12 WebKit Review Bot 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
Comment 13 Ojan Vafai 2011-10-17 14:49:15 PDT
Committed r97654: <http://trac.webkit.org/changeset/97654>
Comment 14 Ojan Vafai 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>
Comment 15 Ojan Vafai 2011-10-17 20:12:11 PDT
Committed r97707: <http://trac.webkit.org/changeset/97707>