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

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

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>

[zalan]

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

[zalan]

Created attachment 234440 [details]
Patch

[Build Bot]

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]

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 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]

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 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]

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 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.

[zalan]

(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.

[zalan]

Created attachment 234596 [details]
Patch

[Simon Fraser (smfr)]

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.

[zalan]

Created attachment 234763 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 234763 [details]
Patch

Clearing flags on attachment: 234763

Committed r171001: <http://trac.webkit.org/changeset/171001>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.