Bug 164261 - Add basic visual/layout viewport support for fixed position layout
Summary: Add basic visual/layout viewport support for fixed position layout
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks: 164260
  Show dependency treegraph
 
Reported: 2016-10-31 18:54 PDT by Simon Fraser (smfr)
Modified: 2016-10-31 22:36 PDT (History)
11 users (show)

See Also:


Attachments
Patch (68.18 KB, patch)
2016-10-31 19:36 PDT, Simon Fraser (smfr)
dino: review+
Details | Formatted Diff | Diff
Patch (67.33 KB, patch)
2016-10-31 20:04 PDT, Simon Fraser (smfr)
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (932.45 KB, application/zip)
2016-10-31 21:10 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (1.16 MB, application/zip)
2016-10-31 21:14 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-yosemite (1.73 MB, application/zip)
2016-10-31 21:21 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews122 for ios-simulator-wk2 (12.64 MB, application/zip)
2016-10-31 21:36 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2016-10-31 18:54:36 PDT
First part of visual viewport support: getting the right rectangles for layout.
Comment 1 Simon Fraser (smfr) 2016-10-31 19:36:53 PDT
Created attachment 293517 [details]
Patch
Comment 2 Dean Jackson 2016-10-31 19:52:10 PDT
Comment on attachment 293517 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=293517&action=review

> Source/WebCore/page/FrameView.cpp:1802
> +    if (visualViewport.y() < layoutViewport.y() || visualViewport.y() < stableLayoutViewportOriginMin.x())

ERROR!!!
you mean stableLayoutViewportOriginMin.y()

> Source/WebCore/page/FrameView.cpp:1837
> +    LOG_WITH_STREAM(Scrolling, stream << "\nFrameView " << this << " updateLayoutViewport()");
> +
> +    LOG_WITH_STREAM(Scrolling, stream << "layoutViewport: " << layoutViewport);

Remove blank.

> Source/WebCore/page/FrameView.cpp:1866
> +    // This isn't visibleContentRect(), because that uses a scaled scroll origin. Confused? Me too.

Me three.

> Source/WebCore/page/FrameView.cpp:1867
> +    FloatRect visibleContentRect = this->visibleContentRect(LegacyIOSDocumentVisibleRect);

I assume that's available on other platforms too?

> Source/WebCore/page/FrameView.cpp:1904
> +    return scrollPositionForFixedPosition(visibleContentRect(), totalContentsSize(), scrollPosition(), scrollOrigin(), frameScaleFactor(), fixedElementsLayoutRelativeToFrame(), scrollBehaviorForFixedElements(), headerHeight(), footerHeight());

The number of arguments here is nuts.

> Source/WebCore/page/FrameView.cpp:2043
> +        IntPoint unscaledScrollOrigin = -unscaledDocumentRect.location();

Do you need the - part here? It confused me because it would flip the x part too. Except you only use this point to make an IntSize, so maybe just call toIntSize(unscaledDocumentRect.location()) directly?

> Source/WebCore/page/Settings.in:280
> +visualViewportEnabled initial=false, setNeedsStyleRecalcInAllFrames=1

Cool. I didn't know you could do this!

> Source/WebCore/platform/Logging.cpp:80
>      WTFInitializeLogChannelStatesFromString(logChannels, logChannelCount, logLevelString().utf8().data());
> +    LogLayout.state = WTFLogChannelOn;
> +    LogScrolling.state = WTFLogChannelOn;

Ooops! Are you trying to annoy everyone?

> Source/WebCore/testing/Internals.cpp:1096
> +    WTFLogAlways("Internals::layoutViewportRect returning %.2f, %.2f, %.2fx%.2f\n",
> +        layoutViewport.x().toFloat(), layoutViewport.y().toFloat(), layoutViewport.width().toFloat(), layoutViewport.height().toFloat());

Oops again.

> LayoutTests/fast/visual-viewport/nonzoomed-rects.html:31
> +            debug('Scrolled to ' + window.scrollX + ', ' + window.scrollY);

Could use `` strings if you felt like it.
Comment 3 Simon Fraser (smfr) 2016-10-31 20:04:22 PDT
Created attachment 293520 [details]
Patch
Comment 4 Build Bot 2016-10-31 21:10:42 PDT
Comment on attachment 293520 [details]
Patch

Attachment 293520 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2418909

New failing tests:
fast/xmlhttprequest/xmlhttprequest-responsetype-sync-request.html
http/tests/websocket/tests/hybi/binary-type.html
Comment 5 Build Bot 2016-10-31 21:10:45 PDT
Created attachment 293526 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 6 Build Bot 2016-10-31 21:14:24 PDT
Comment on attachment 293520 [details]
Patch

Attachment 293520 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2418921

New failing tests:
fast/xmlhttprequest/xmlhttprequest-responsetype-sync-request.html
http/tests/websocket/tests/hybi/binary-type.html
Comment 7 Build Bot 2016-10-31 21:14:27 PDT
Created attachment 293527 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 8 Build Bot 2016-10-31 21:21:39 PDT
Comment on attachment 293520 [details]
Patch

Attachment 293520 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2418937

New failing tests:
fast/xmlhttprequest/xmlhttprequest-responsetype-sync-request.html
http/tests/websocket/tests/hybi/binary-type.html
Comment 9 Build Bot 2016-10-31 21:21:42 PDT
Created attachment 293528 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 10 Build Bot 2016-10-31 21:36:15 PDT
Comment on attachment 293520 [details]
Patch

Attachment 293520 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2418939

New failing tests:
fast/xmlhttprequest/xmlhttprequest-responsetype-sync-request.html
http/tests/websocket/tests/hybi/binary-type.html
fast/visual-viewport/zoomed-fixed.html
Comment 11 Build Bot 2016-10-31 21:36:19 PDT
Created attachment 293529 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 12 Simon Fraser (smfr) 2016-10-31 22:36:11 PDT
https://trac.webkit.org/r208213