Bug 137981 - CSS 3.0 letterSpacing property is defined as normal | <length> but in WebKit it is defined as normal | float
Summary: CSS 3.0 letterSpacing property is defined as normal | <length> but in WebKit ...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-10-22 14:44 PDT by Said Abou-Hallawa
Modified: 2014-10-23 17:20 PDT (History)
2 users (show)

See Also:


Attachments
Patch (14.13 KB, patch)
2014-10-22 17:41 PDT, Said Abou-Hallawa
sam: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.