Bug 98677 - Replace 2 uses of updateLogicalHeight with computeLogicalHeight
Summary: Replace 2 uses of updateLogicalHeight with computeLogicalHeight
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks: 96804
  Show dependency tree / graph
 
Reported: 2012-10-08 11:42 PDT by Tony Chang
Modified: 2012-10-08 15:35 PDT (History)
9 users (show)

See Also:

Attachments
Patch (5.71 KB, patch)
2012-10-08 11:51 PDT, Tony Chang 		no flags Details | Formatted Diff | Diff
Patch for landing (5.76 KB, patch)
2012-10-08 12:20 PDT, Tony Chang 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Chang 2012-10-08 11:42:53 PDT 
Replace 2 uses of updateLogicalHeight with computeLogicalHeight
Comment 1 Tony Chang 2012-10-08 11:51:43 PDT 
Created attachment 167576 [details]
Patch
Comment 2 Ojan Vafai 2012-10-08 11:58:34 PDT 
Comment on attachment 167576 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167576&action=review

> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:115
> +void RenderSVGForeignObject::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop, LogicalExtentComputedValues& computedValues) const

I assume foreignObject is forced into horizontal-tb?

Not really related to this patch, but I feel like we should have asserts at the top of all the layout/computeLogicalHeight/computeLogicalWidth methods for RenderObject subclasses that don't support vertical writing mode. Either that, or a FIXME to support it.
Comment 3 Tony Chang 2012-10-08 12:10:32 PDT 
Comment on attachment 167576 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167576&action=review

>> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:115
>> +void RenderSVGForeignObject::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop, LogicalExtentComputedValues& computedValues) const
> 
> I assume foreignObject is forced into horizontal-tb?
> 
> Not really related to this patch, but I feel like we should have asserts at the top of all the layout/computeLogicalHeight/computeLogicalWidth methods for RenderObject subclasses that don't support vertical writing mode. Either that, or a FIXME to support it.

Oh, no, they're not forced into horizontal-tb.  Let me see if I can make a test case for that.
Comment 4 Tony Chang 2012-10-08 12:20:23 PDT 
Created attachment 167584 [details]
Patch for landing
Comment 5 Tony Chang 2012-10-08 12:21:23 PDT 
(In reply to comment #3)
> Oh, no, they're not forced into horizontal-tb.  Let me see if I can make a test case for that.

Actually, I can't seem to get an inline svg to render in vertical-rl.  I'm not sure what's supposed to happen here.
Comment 6 WebKit Review Bot 2012-10-08 14:16:25 PDT 
Comment on attachment 167584 [details]
Patch for landing

Clearing flags on attachment: 167584

Committed r130686: <http://trac.webkit.org/changeset/130686>
Comment 7 WebKit Review Bot 2012-10-08 14:16:29 PDT 
All reviewed patches have been landed.  Closing bug.