Bug 99767

Summary: Change baselinePosition and maxAscent/maxDescent to int
Product: WebKit Reporter: Emil A Eklund <eae>
Component: Layout and RenderingAssignee: Emil A Eklund <eae>
Status: RESOLVED FIXED    
Severity: Normal CC: dbarton, eric, gyuyoung.kim, leviw, mifenton, rakuco, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Emil A Eklund
Reported 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.
Attachments
Patch (462.30 KB, patch)
2012-10-18 15:49 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Patch (466.07 KB, patch)
2012-10-18 16:03 PDT, Emil A Eklund 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Emil A Eklund
Comment 1 2012-10-18 15:49:48 PDT
Created attachment 169492 [details] Patch
Emil A Eklund
Comment 2 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.
WebKit Review Bot
Comment 3 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.
Early Warning System Bot
Comment 4 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
Early Warning System Bot
Comment 5 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
Emil A Eklund
Comment 6 2012-10-18 16:03:40 PDT
Created attachment 169498 [details] Patch
WebKit Review Bot
Comment 7 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.
Emil A Eklund
Comment 8 2012-10-19 09:33:03 PDT
Downstream chromium bug: http://code.google.com/p/chromium/issues/detail?id=153268
Levi Weintraub
Comment 9 2012-10-22 10:21:45 PDT
Comment on attachment 169498 [details] Patch LGTM, but keep the Mac bots green :)
Emil A Eklund
Comment 10 2012-10-22 12:05:20 PDT
Committed r132112: <http://trac.webkit.org/changeset/132112>
Note You need to log in before you can comment on or make changes to this bug.