Bug 99767 - Change baselinePosition and maxAscent/maxDescent to int
Summary: Change baselinePosition and maxAscent/maxDescent to int
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: Emil A Eklund
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-18 15:38 PDT by Emil A Eklund
Modified: 2012-10-22 12:05 PDT (History)
8 users (show)

See Also:


Attachments
Patch (462.30 KB, patch)
2012-10-18 15:49 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (466.07 KB, patch)
2012-10-18 16:03 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emil A Eklund 2012-10-18 15:38:54 PDT
Currently baselinePostion, maxAscent and maxDescent are LayoutUnits while ascent, descent and m_lineHeight are ints. This can lead to subtle alignment and rounding problems.

Change baselinePosition and maxAscent/maxDescent to int to avoid these issues.
Comment 1 Emil A Eklund 2012-10-18 15:49:48 PDT
Created attachment 169492 [details]
Patch
Comment 2 Emil A Eklund 2012-10-18 15:50:59 PDT
EWS might complain about indentation in InlineFlowBox. This is expected as I'm leaving the indentation as is.
Comment 3 WebKit Review Bot 2012-10-18 15:54:53 PDT
Attachment 169492 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/WebCore/rendering/InlineFlowBox.cpp:506:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/rendering/InlineFlowBox.h:178:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/rendering/InlineFlowBox.h:181:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 3 in 65 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Early Warning System Bot 2012-10-18 15:58:33 PDT
Comment on attachment 169492 [details]
Patch

Attachment 169492 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14400986
Comment 5 Early Warning System Bot 2012-10-18 15:59:18 PDT
Comment on attachment 169492 [details]
Patch

Attachment 169492 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14463170
Comment 6 Emil A Eklund 2012-10-18 16:03:40 PDT
Created attachment 169498 [details]
Patch
Comment 7 WebKit Review Bot 2012-10-18 16:11:55 PDT
Attachment 169498 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/WebCore/rendering/InlineFlowBox.cpp:506:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/rendering/InlineFlowBox.h:178:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/rendering/InlineFlowBox.h:181:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 3 in 71 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Emil A Eklund 2012-10-19 09:33:03 PDT
Downstream chromium bug: http://code.google.com/p/chromium/issues/detail?id=153268
Comment 9 Levi Weintraub 2012-10-22 10:21:45 PDT
Comment on attachment 169498 [details]
Patch

LGTM, but keep the Mac bots green :)
Comment 10 Emil A Eklund 2012-10-22 12:05:20 PDT
Committed r132112: <http://trac.webkit.org/changeset/132112>