Bug 42863

Summary: REGRESSION (r60104): Zoom level is unexpectedly reset on page reload
Product: WebKit Reporter: Xiaomei Ji <xji>
Component: Page LoadingAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, bdakin, beidson, darin, dbates, dev+webkit, dglazkov, eric, fishd, mihaip, sullivan, webkit.review.bot
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.6   
URL: http://www.apple.com/startpage
Bug Depends on: 46093    
Bug Blocks:    
Attachments:
Description Flags
Patch
sam: review+
Layout Test none

Description Xiaomei Ji 2010-07-22 17:45:44 PDT
Version: WebKit r63854 (was built on 21 July 2010 and is a 27.9 MB download)

Steps to reproduce:
1. open any webpage, for example: www.apple.com/startpage
2. continue to zoom in the page to the largest. 
4. reload the page

The page appears to be reset to normal size.
But the zoom level is preserved as the largest zoom level.  (the "zoom in" is greyed, and press zoom in wont work. But press zoom out will work which actually decreases the font size from the one before reload, and the font size appears to be bigger comparing to the one before zoom out).


Tested in: OSX 10.6
Comment 1 Alexey Proskuryakov 2010-07-24 17:12:23 PDT
In shipping Safari/WebKit 5.0, the page stays zoomed in.
Comment 2 Alexey Proskuryakov 2010-07-24 17:12:48 PDT
<rdar://problem/8231917>
Comment 3 Matt Lilek 2010-09-14 12:27:11 PDT
*** Bug 41573 has been marked as a duplicate of this bug. ***
Comment 4 Matt Lilek 2010-09-14 12:27:23 PDT
This regressed in http://trac.webkit.org/changeset/60104
Comment 5 Darin Adler 2010-09-14 12:58:46 PDT
I’m surprised that moving these functions caused this. I guess a new FrameView is created and does not inherit the zoom level from the older FrameView?
Comment 6 Darin Adler 2010-09-17 15:34:16 PDT
Created attachment 67963 [details]
Patch
Comment 7 Daniel Bates 2010-09-17 17:13:23 PDT
We should be able to test this change by using a render tree dump/pixel test and eventSender.zoomPageIn().
Comment 8 Daniel Bates 2010-09-17 17:15:04 PDT
Created attachment 67977 [details]
Layout Test

I was not sure where to place this test case. I noticed other test cases that used eventSender.zoomInPage() in fast/css. So, I put the test case in this directory. I am open to suggestions.
Comment 9 Daniel Bates 2010-09-17 17:16:10 PDT
(In reply to comment #8)
> Created an attachment (id=67977) [details]
> Layout Test
> 
> I was not sure where to place this test case. I noticed other test cases that used eventSender.zoomInPage() in fast/css. So, I put the test case in this directory. I am open to suggestions.

I mean't to write eventSender.zoomPageIn() instead of eventSender.zoomInPage().
Comment 10 Darin Adler 2010-09-17 17:22:54 PDT
Comment on attachment 67977 [details]
Layout Test

I’m surprised this works, but it’s good news that it does.
Comment 11 Darin Adler 2010-09-17 17:32:48 PDT
Committed r67762: <http://trac.webkit.org/changeset/67762>
Comment 12 WebKit Review Bot 2010-09-17 17:43:20 PDT
http://trac.webkit.org/changeset/67762 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release
Comment 13 Dimitri Glazkov (Google) 2010-09-17 19:48:57 PDT
(In reply to comment #12)
> http://trac.webkit.org/changeset/67762 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release

All Chromium builds are borked :(
Comment 14 Eric Seidel (no email) 2010-09-18 03:21:25 PDT
This appears to have torched the tree. :(
Comment 15 Mihai Parparita 2010-09-18 10:10:24 PDT
I've put up a patch at bug 46040 that should at least fix the Snow Leopard bot (and unblock the commit queue) if someone wouldn't mind r+-ing and landing it for me.
Comment 16 Darin Adler 2010-09-18 16:00:08 PDT
Sorry about the incorrect logic and build breakage. Hope everything’s fixed now. Thanks to those who helped patch things up.