[webkit-reviews] review granted: [Bug 98571] [Sub pixel layout] Fast-path iframe scrolling can picks up an extra pixel : [Attachment 167574] Patch

[bugzilla-daemon at webkit.org]

Emil A Eklund <eae at chromium.org> has granted Levi Weintraub
<leviw at chromium.org>'s request for review:
Bug 98571: [Sub pixel layout] Fast-path iframe scrolling can picks up an extra
pixel
https://bugs.webkit.org/show_bug.cgi?id=98571

Attachment 167574: Patch
https://bugs.webkit.org/attachment.cgi?id=167574&action=review

------- Additional Comments from Emil A Eklund <eae at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=167574&action=review


>>> Source/WebCore/dom/Element.cpp:542
>>> +		 renderBoxModelObject()->absoluteQuads(quads, 0);
>> 
>> Why are you explicitly passing zero (NULL) as wasFixed here?
> 
> This is because we don't want any of the feature flags. We could have a named
zero constant to specify no flags?

Either that or adding the name of the field as a comment (just like we did in
some places for the boolean parameters before).

renderBoxModelObject()->absoluteQuads(quads, 0 /* mode */);

>>> Source/WebCore/dom/MouseRelatedEvent.cpp:169
>>> +		 FloatPoint localPos = r->absoluteToLocal(absoluteLocation(),
UseTransforms | SnapOffsetForTransforms);
>> 
>> SnapOffsetForTransforms seems to be used in most places, how about making it
the default?
> 
> SnapOffsetForTransforms is, UseTransforms isn't as this was the previous
default. It does honestly seem like using transforms should be the default and
callers should be specifying explicitly that they *don't* want transforms.
Perhaps that's worthwhile here or in a future patch (since it would reverse
logic).

Lets keep this change as small as we can and have that in a future patch.