Bug 134651

Summary:

Subpixel layout: return integral results for offset*, client*, scroll* by default.

Product:

WebKit

Reporter:

alan baradlay <zalan>

Component:

Layout and Rendering

Assignee:

alan baradlay <zalan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

buildbot, cmarcelo, commit-queue, esprehn+autocc, kangil.han, rniwa, simon.fraser

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion	none
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2	none
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion	none
Patch	none
Patch	none

alan baradlay

Reported 2014-07-05 09:12:39 PDT

Keep the double return type but round/truncate the return value. (see bug 132895) <rdar://problem/17435776> Fractional values break number of sites/JS frameworks: 1. gmail.com (inbox items don’t show up when certain languages are set), 2. "Setup Instructions" on icloud.com ellipsized -> tight design. <rdar://problem/17028563> 3. can't into fidelity.netbenefits.com -> RequireJS fails to make some content fit due to offsetWidth (fractional) vs. scrollWidth (integral) value compare. <rdar://problem/17227701> 4. Text labels are truncated with elipses @ Find My iPhone @ icloud.com -> similar to 2, tight design. <rdar://problem/17338131> 5. Table is missing from a page at bethematch.org -> regex fails to handle fractional values (/.*if_height=(\d+)(?:&|$)/) <rdar://problem/17480566>

Attachments
Patch (117.06 KB, patch) 2014-07-05 10:09 PDT, alan baradlay	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (642.04 KB, application/zip) 2014-07-05 11:40 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (609.28 KB, application/zip) 2014-07-05 12:15 PDT, Build Bot	no flags	Details
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion (642.52 KB, application/zip) 2014-07-05 12:47 PDT, Build Bot	no flags	Details
Patch (125.59 KB, patch) 2014-07-08 14:23 PDT, alan baradlay	no flags	Details Formatted Diff Diff
Patch (125.65 KB, patch) 2014-07-11 07:49 PDT, alan baradlay	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

alan baradlay

Comment 1 2014-07-05 09:55:17 PDT

1. FireFox: (integral values, no fractional support) - FireFox has been advocating Element.getBoundingClientRect(1) and getBoxQuads(2) to access to fractional values instead of Element.offset* etc. 2. IE: (integral values by default, opt-in for fractional values) - IE has the fractional behavior behind document.msCSSOMElementFloatMetrics property.(3) - It’s off by default -> integral values (4). 3. Chrome (integral values by default for now) - Chrome had been returning fractional values (5) by default until recently. - Chrome turned fractional values off in r176545 (6): -> "Due to breaking a number of high profile sites we've been forced to revert this change. I plan to try again in M38. “. (1) https://bugzilla.mozilla.org/show_bug.cgi?id=825607 (2) https://hacks.mozilla.org/2014/03/introducing-the-getboxquads-api/ (3) http://blogs.msdn.com/b/ie/archive/2012/02/17/sub-pixel-rendering-and-the-css-object-model.aspx?Redirected=true (4) http://msdn.microsoft.com/en-us/library/ie/hh673543(v=vs.85).aspx (5) https://src.chromium.org/viewvc/blink?revision=173426&view=revision (6) http://src.chromium.org/viewvc/blink?view=revision&revision=176545

alan baradlay

Comment 2 2014-07-05 10:09:27 PDT

Created attachment 234440 [details] Patch

Build Bot

Comment 3 2014-07-05 11:40:03 PDT

Comment on attachment 234440 [details] Patch Attachment 234440 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5563074249490432 New failing tests: editing/selection/drag-start-event-client-x-y.html fast/block/positioning/offsetLeft-offsetTop-multicolumn.html editing/selection/mixed-editability-10.html editing/selection/collapse-selection-in-bidi.html

Build Bot

Comment 4 2014-07-05 11:40:06 PDT

Created attachment 234443 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Build Bot

Comment 5 2014-07-05 12:15:07 PDT

Comment on attachment 234440 [details] Patch Attachment 234440 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5484350183309312 New failing tests: editing/selection/drag-start-event-client-x-y.html fast/block/positioning/offsetLeft-offsetTop-multicolumn.html editing/selection/collapse-selection-in-bidi.html

Build Bot

Comment 6 2014-07-05 12:15:11 PDT

Created attachment 234446 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Build Bot

Comment 7 2014-07-05 12:47:23 PDT

Comment on attachment 234440 [details] Patch Attachment 234440 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4681424300933120 New failing tests: editing/selection/drag-start-event-client-x-y.html fast/block/positioning/offsetLeft-offsetTop-multicolumn.html editing/selection/mixed-editability-10.html editing/selection/collapse-selection-in-bidi.html

Build Bot

Comment 8 2014-07-05 12:47:25 PDT

Created attachment 234447 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5

Darin Adler

Comment 9 2014-07-06 17:19:44 PDT

Comment on attachment 234440 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234440&action=review > Source/WebCore/ChangeLog:3 > + Subpixel layout: return integral results for offset*, client*, scroll* by default. Longer term, why are we even keeping this boolean setting? Seems we should not be paying any runtime overhead for this experiment. We now know that the non-integral values will need to come from other APIs.

alan baradlay

Comment 10 2014-07-07 14:00:45 PDT

(In reply to comment #9) > (From update of attachment 234440 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234440&action=review > > > Source/WebCore/ChangeLog:3 > > + Subpixel layout: return integral results for offset*, client*, scroll* by default. > > Longer term, why are we even keeping this boolean setting? Seems we should not be paying any runtime overhead for this experiment. We now know that the non-integral values will need to come from other APIs. We need to keep this runtime setting around until after we introduced those new API as there's a Mac app that depends on these fractional values.

alan baradlay

Comment 11 2014-07-08 14:23:19 PDT

Created attachment 234596 [details] Patch

Simon Fraser (smfr)

Comment 12 2014-07-10 10:34:25 PDT

Comment on attachment 234596 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234596&action=review > Source/WebCore/dom/Element.cpp:767 > + double clientTop = subpixelMetricsEnabled(renderer->document()) ? renderer->clientTop().toDouble() : roundToInt(renderer->clientTop()); double -> LayoutUnit for all of these.

alan baradlay

Comment 13 2014-07-11 07:49:16 PDT

Created attachment 234763 [details] Patch

WebKit Commit Bot

Comment 14 2014-07-11 08:28:05 PDT

Comment on attachment 234763 [details] Patch Clearing flags on attachment: 234763 Committed r171001: <http://trac.webkit.org/changeset/171001>

WebKit Commit Bot

Comment 15 2014-07-11 08:28:10 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.