Bug 137981

Summary: CSS 3.0 letterSpacing property is defined as normal | <length> but in WebKit it is defined as normal | float
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: CSSAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED INVALID    
Severity: Normal CC: sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch sam: review-

Description Said Abou-Hallawa 2014-10-22 14:44:26 PDT
We need to handle the letter-spacing property very similar to the word-spacing property.  The only difference is word-spacing can be a percentage of the space width.  The letter spacing can only be normal | <length>.

Link ot w3c specs is: http://www.w3.org/TR/css3-text/#letter-spacing
Comment 1 Said Abou-Hallawa 2014-10-22 15:47:54 PDT
<rdar://problem:18743210>
Comment 2 Said Abou-Hallawa 2014-10-22 15:48:34 PDT
<rdar://problem/18743210>
Comment 3 Said Abou-Hallawa 2014-10-22 17:41:22 PDT
Created attachment 240314 [details]
Patch
Comment 4 Sam Weinig 2014-10-23 12:44:29 PDT
Comment on attachment 240314 [details]
Patch

This needs tests.
Comment 5 Radar WebKit Bug Importer 2014-10-23 12:45:15 PDT
<rdar://problem/18754113>
Comment 6 Sam Weinig 2014-10-23 12:45:20 PDT
(In reply to comment #4)
> Comment on attachment 240314 [details]
> Patch
> 
> This needs tests.

More specifically, I don't understand why we need this patch if it doesn't change behavior.
Comment 7 Radar WebKit Bug Importer 2014-10-23 12:45:36 PDT
<rdar://problem/18754116>
Comment 8 Said Abou-Hallawa 2014-10-23 16:35:11 PDT
Here is the history behind this bug:

1. I was working on https://bugs.webkit.org/show_bug.cgi?id=129350 and I was asked why setting wordSpacing  to huge percentage value causes a security assertion to fire but with letter spacing no assertion is fired.

2. I looked at the code and I found out letterSpacing is defined of type float while wordSpacing is defined of type Length.  The Length handler of the wordSpacing was not clamping the percentage value to some min/max values.

3. Then I asked why letterSpacing is of type float while wordSpacing is of type Length.  The answer I got was we should handle both very similar expect for the percentage case which can only be applied to the wordSpacing.  So I added the type change float -> Length of the letterSpacing to the patch of 129350.

4. But for the combined patch uploaded to 129350, I was asked to split the two changes into two separate patches.  So I logged this bug and I split the patches into two.

5. The attached patch was challenged for why it needs to be taken.  So I looked at the history of the wordSpacing code and I found out it was defined of type float until css3.0 decided to support percentage for the wordSpacing property.  So to fix https://bugs.webkit.org/show_bug.cgi?id=126674, the wordSpacing was converted from float to Length.

6. Since letterSpacing can't be a percentage, we need not to convert it from float to Length.  So I am resolving this bug as INVALID.