Scrolling must be done after the layout when doing full page zoom

• Component: Scrolling
• Status: RESOLVED FIXED

[Tomoki Imai]

How to reproduce:

- Open a long page (for example http://www.rakuten.co.jp/)
- Scroll to the bottom
- Zoom in

Actual result:

Compared to the other browsers (Chrome, Firefox), the scroll position after zoom is different from the original position.

Expected result:

The relative scroll position should be retained as much as possible.

Environment:
- WinCairo MiniBrowser (trunk r277317)
- WebKitGTK MiniBrowser (trunk r277317)
- Safari 13.1 (15609.1.20.111.8)

Note:
In the current code, the updating the scrolling position is done before the re-layout.
The scrolling position is updated with the scale, but it is rounded by the original page height.
https://github.com/WebKit/WebKit/blob/b37910cc0712363914a6d3d2f655db1c9c892f55/Source/WebCore/page/Frame.cpp#L964-L984

[Tomoki Imai]

Created attachment 428451 [details]
Patch

Patch to change the scrolling timing to the after layout.

[Simon Fraser (smfr)]

When you say "Zoom in", how are you zooming?

[Simon Fraser (smfr)]

When I scroll to the bottom on http://www.rakuten.co.jp/, then pinch-zoom to zoom in the scroll position stays in the correct place.

Maybe there's a different issue on Windows?

[Tomoki Imai]

Created attachment 428453 [details]
Patch

Rebased on the trunk

[Tomoki Imai]

(In reply to Simon Fraser (smfr) from comment #2)
> When you say "Zoom in", how are you zooming?

In Safari, we tried "View" -> "Zoom In" in the menu bar which changes "ZoomFactor". (I'll post a video later)

(In reply to Simon Fraser (smfr) from comment #3)
> When I scroll to the bottom on http://www.rakuten.co.jp/, then pinch-zoom to
> zoom in the scroll position stays in the correct place.
> 
> Maybe there's a different issue on Windows?

I didn't try the pinch-zoom, but I guess it changes the "ScaleFactor" not "ZoomFactor".
The updating scrolling position code is shared among the ports, so I believe it's not only for Windows/GTK.

[Simon Fraser (smfr)]

OK, I do see a flash of incorrect scroll position when zooming with Command-+ after scrolling down (it jumps up, then back down again).

[Tomoki Imai]

Created attachment 428457 [details]
Reproduction in Safari 13.1

This video shows what's happened in Safari 13.1.
(Sorry for using the old Safari...)

00:00 At first, the zoom factor is the default value (probably 1). We don't see the cart icon.
00:09 Select "Zoom In", and we now see the cart icon (which we don't see in Chrome/Firefox).
01:12 Even after selecting "Zoom Out" to make the zoom factor the original value, it doesn't retain the original scroll position.

[Tomoki Imai]

(In reply to Simon Fraser (smfr) from comment #6)
> OK, I do see a flash of incorrect scroll position when zooming with
> Command-+ after scrolling down (it jumps up, then back down again).

Thanks for trying!
Yes, Command-+ also should reproduce the issue because it's a shortcut for "View" -> "Zoom In".

[Tomoki Imai]

Created attachment 428465 [details]
Patch

Fixed LayoutTest expected result (Added newlines).

[Radar WebKit Bug Importer]

<rdar://problem/78239415>

[EWS]

Committed r277775 (237936@main): <https://commits.webkit.org/237936@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 428465 [details].

[Tomoki Imai]

Thank you for your review!