RESOLVED FIXED 171140
[iOS] REGRESSION (r209409): getBoundingClientRect is wrong for fixed-position elements in resize/orientationchange
https://bugs.webkit.org/show_bug.cgi?id=171140
Summary [iOS] REGRESSION (r209409): getBoundingClientRect is wrong for fixed-position...
Tim Horton
Reported 2017-04-21 13:43:57 PDT
Created attachment 307789 [details] testcase Steps to Reproduce: 1. Open attached test page in Safari on iOS 10.3+. 2. Rotate. Expected: all three borders should line up Actual: the borders (green and blue) that are derived from the bounding client rect of the fixed-position element inside onresize/onorientationchange don't line up with the fixed-position element's border.
Attachments
testcase (1.39 KB, text/html)
2017-04-21 13:43 PDT, Tim Horton
no flags
Patch (11.77 KB, patch)
2017-04-25 14:21 PDT, Tim Horton
simon.fraser: review-
Patch for EWS (25.83 KB, patch)
2017-05-05 20:06 PDT, Simon Fraser (smfr)
no flags
Patch (31.84 KB, patch)
2017-05-06 21:24 PDT, Simon Fraser (smfr)
no flags
Patch (31.15 KB, patch)
2017-05-07 10:49 PDT, Simon Fraser (smfr)
no flags
Radar WebKit Bug Importer
Comment 1 2017-04-21 13:52:24 PDT
Simon Fraser (smfr)
Comment 2 2017-04-21 14:09:16 PDT
We need a way to test rotation.
Tim Horton
Comment 3 2017-04-21 14:41:14 PDT
I don't think testing this is blocked on "a way to test rotation", I'm pretty sure this is just a resize bug. And we have ways to test that.
Tim Horton
Comment 4 2017-04-24 12:55:10 PDT
Tim Horton
Comment 5 2017-04-24 17:42:53 PDT
(And ToT is OK with Visual Viewports disabled)
Tim Horton
Comment 6 2017-04-24 20:40:26 PDT
It turns out (after hours of debugging) that the problem here is simple: WebPage::dynamicViewportSizeUpdate was not updated to set the layoutViewportOverrideRect instead of the customFixedPositionLayoutRect like WebPage::updateVisibleContentRects was. I have tested after making this change and things are better; I'll make some tests and post a patch.
Tim Horton
Comment 7 2017-04-25 14:21:55 PDT
Tim Horton
Comment 8 2017-04-25 14:39:06 PDT
Comment on attachment 308144 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308144&action=review > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:-2992 > - frameView.setCustomFixedPositionLayoutRect(fixedPositionLayoutRect); We may want to try to clean this up and share more code between dynamicViewportSizeUpdate and updateVisibleContentRects, but that is a much bigger change that I don't want to make at the moment.
Simon Fraser (smfr)
Comment 9 2017-04-26 11:05:11 PDT
Comment on attachment 308144 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308144&action=review > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:2994 > + frameView.setLayoutViewportOverrideRect(LayoutRect(fixedPositionLayoutRect)); This is wrong, because fixedPositionLayoutRect comes from frameView.viewportConstrainedObjectsRect(), which is for the old-style fixed behavior, not the visual viewport behavior. We need to get this rect from _page->computeCustomFixedPositionRect().
Simon Fraser (smfr)
Comment 10 2017-05-05 20:06:37 PDT
Created attachment 309262 [details] Patch for EWS
Simon Fraser (smfr)
Comment 11 2017-05-06 21:24:02 PDT
Simon Fraser (smfr)
Comment 12 2017-05-07 10:49:36 PDT
WebKit Commit Bot
Comment 13 2017-05-07 14:23:21 PDT
Comment on attachment 309323 [details] Patch Clearing flags on attachment: 309323 Committed r216352: <http://trac.webkit.org/changeset/216352>
WebKit Commit Bot
Comment 14 2017-05-07 14:23:23 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.