Bug 48085

Summary: Make basic vertical text painting work.
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 46123    
Attachments:
Description Flags
Patch
darin: review+
Patch that adds new results from Takano's change.
none
New ugly results from Takano's and my patch combining. none

Description Dave Hyatt 2010-10-21 12:10:14 PDT
Make basic vertical text painting work.  This includes the text itself, underlines, overlines, line-throughs and shadows.

Selection would draw correctly if hit testing worked well or if invalidation worked well. :)
Comment 1 Dave Hyatt 2010-10-21 12:12:28 PDT
Created attachment 71469 [details]
Patch
Comment 2 Darin Adler 2010-10-21 12:39:37 PDT
Comment on attachment 71469 [details]
Patch

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

> WebCore/WebCore.xcodeproj/project.pbxproj:-21155
> -			developmentRegion = English;

You need to update your Xcode with Software Update. The newer version doesn’t remove this, and you should not check in this project change.

> WebCore/rendering/InlineFlowBox.cpp:-708
> -            paintTextDecorations(paintInfo, tx, ty, false);

How does this work now? Is there other code that always paints text decorations now that used to do it only conditionally?

> WebCore/rendering/InlineTextBox.cpp:433
> +        context->rotate(deg2rad(90.));

We normally would just use 90 here instead of "90." but maybe that will cause overloading problems with the deg2rad function. Also, this will call deg2rad at runtime -- maybe we should include a constant you can use for this. Maybe a helper function to rotate 90 degrees would be cleaner?
Comment 3 Dave Hyatt 2010-10-21 13:16:28 PDT
(In reply to comment #2)
>
> > WebCore/rendering/InlineFlowBox.cpp:-708
> > -            paintTextDecorations(paintInfo, tx, ty, false);
> 
> How does this work now? Is there other code that always paints text decorations now that used to do it only conditionally?
> 

We have two complete text decoration drawing code paths.  One is for quirks mode and one is for strict mode.  CSS2.1 has defined a third model that is not compatible with either of our models (yay).  However our quirks mode model is much closer to what CSS2.1 wants.  In addition CSS2.1 states that "older UAs that predate CSS2.1 are still conformant" if we implement text-decoration the way our quirks mode model does.

This patch is removing the strict mode text decoration code path and just making the quirks mode one the only path we use.
Comment 4 Dave Hyatt 2010-10-21 14:14:25 PDT
Created attachment 71489 [details]
Patch that adds new results from Takano's change.
Comment 5 Dave Hyatt 2010-10-21 14:17:24 PDT
Fixed in r70263.
Comment 6 Dave Hyatt 2010-10-21 14:28:01 PDT
Created attachment 71493 [details]
New ugly results from Takano's and my patch combining.

Patch
Comment 7 Eric Seidel (no email) 2010-10-21 14:49:07 PDT
This broke snow leopard.
Comment 8 Ryosuke Niwa 2010-10-21 18:15:36 PDT
(In reply to comment #7)
> This broke snow leopard.

Fixed in http://trac.webkit.org/changeset/70281.  Please verify the fix.