Bug 164261

Summary: Add basic visual/layout viewport support for fixed position layout
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, cmarcelo, commit-queue, esprehn+autocc, glenn, jamesr, kondapallykalyan, luiz, rniwa, simon.fraser, tonikitoo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 164260    
Attachments:
Description Flags
Patch
dino: review+
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews104 for mac-yosemite-wk2
none
Archive of layout-test-results from ews112 for mac-yosemite
none
Archive of layout-test-results from ews122 for ios-simulator-wk2 none

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