Closed Bug 405483 Opened 17 years ago Closed 17 years ago

When zooming: web sites not centered, no scrollbars

Categories

(Toolkit :: Preferences, defect, P1)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
mozilla1.9beta2

People

(Reporter: polidobj, Assigned: myk)

References

()

Details

(Keywords: regression)

Attachments

(2 files)

Some sites are off center and don't have scrollbars when zoomed and reloaded. 

1. Goto a site like the URL given.
2. Zoom the site.  I was zooming in twice. I believe this is needed to invoke the site pref code. 
3. Reload the page from server (Ctrl-F5).

1126 0051 broke 
1126 0034 ok 

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1196066040&maxdate=1196067059
Flags: blocking1.9?
Yup, I see this too.  Investigating...
For me it is not necessary to reload. Any link within the site will do too.
Example: In the mozillazine forums go to quick reply, zoom in and sent the post. The result page is already without scrollbars.
Reseting zoom (CTRL-0) and reloading (CTRL-F5) brings them back.
Reloading is not necessary if you already have a site pref set for that site.  But starting with a clean profile you need to get a site pref set.  And those were the simplest steps to do that.  
This is being caused by the changes to nsPresContext.cpp from bug 385224.  Looking into why...
If you zoom out then click a link then you get the new page only taking up a smaller portion of the normal content area.
Better back that part out then. Should be able to back the nsPresContext changes out and leave the rest in.
-> myk
Assignee: nobody → myk
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1
Target Milestone: --- → mozilla1.9 M10
Although I don't understand the exact issue yet, the bug seems to be caused by the devicecontext and the prescontext being out of sync on prescontext initialization when the previous page has a non default zoom, since the prescontext constructor sets its mFullZoom to the default value (1.0), while the devicecontext's mPixelScale and mAppUnitsPerDevPixel retain their non-default values based on the zoom from the previous page.

Before the checkin for bug 385224, they quickly got back in sync when DocumentViewerImpl::InitPresentationStuff called nsPresContext::SetFullZoom with the default zoom value and nsPresContext::SetFullZoom then called nsThebesDeviceContext::SetPixelScale, even though the new (default) zoom value was the same as the existing (default) one.

Now, however, nsPresContext::SetFullZoom doesn't do anything if the new value is the same as the existing one, so the two contexts stay out of sync until later in the process, when some caller (not sure what yet) calls nsPresContext::SetFullZoom again with the previous page's non-default zoom value.

Reverting the change to nsPresContext.cpp from bug 385224 fixes the bug, but it seems like it would be preferable to explicitly synchronize the two contexts on prescontext initialization, so here's a patch that does that.

Note: it's not clear to me why nsPresContext::SetFullZoom gets called twice, first by DocumentViewerImpl::InitPresentationStuff and then by some other caller (not sure what yet), before being called a *third* time on location change by the front-end TextZoom controller, which sets the zoom to the site-specific value.

It seems like only the third of these calls would be necessary or desirable, and there might be a perf impact here for pages where the default value, the value for the previous page, and the value for the current page are all different (and if either of the first two calls trigger a reflow).  But that's another bug.
Status: NEW → ASSIGNED
Attachment #290315 - Flags: review?(roc)
Comment on attachment 290315 [details] [diff] [review]
patch v1: fixes problem

r+sr=me
Attachment #290315 - Flags: review?(roc) → review+
Checking in layout/base/nsPresContext.cpp;
/cvsroot/mozilla/layout/base/nsPresContext.cpp,v  <--  nsPresContext.cpp
new revision: 3.337; previous revision: 3.336
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
verified that this problem is fixed.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007112705 Minefield/3.0b2pre ID:2007112705

But my content prefs seem to be out of whack.  If I open a new tab from an existing tab with them both being the same domain then the new tab is at the default zoom level instead of the level I have in the existing tab.  And If I change the zoom in one of those tabs it doesn't affect the other tab.  I checked in a new profile and this doesn't happen there.  So I'll see if it sorts itself out.  If not I'll delete my content-prefs.sqlite file and see what happens.  
Status: RESOLVED → VERIFIED
In the console I see:
Error: this._cps is undefined
Source File: chrome://browser/content/browser.js
Line: 5945

Warning: reference to undefined property this._cps
Source File: chrome://browser/content/browser.js
Line: 5945
That error would explain why content prefs don't work for you, since the TextZoom controller needs a reference to the content pref service to get and set prefs.  But I can't reproduce the problem here on Mac and Linux.
Attached file problem file
Well the sqlite file is the problem.  I removed it and things work as expected.  I put that file in another profile and it had the problems too.  I also saw the startup error to go with this:

Error: Failed to init content pref service:
[Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js :: FullZoom_get__cps :: line 5822"  data: no]
Source File: chrome://browser/content/browser.js
Line: 857

From browsing the file I guess it's because there's also left over text zoom prefs in there.  I'm attaching the file in case it's worth looking at.  I just wonder why it shows up now instead of when the switch was made to page zoom.  IRC there are no site prefs in FF2 so I guess upgrading users would not encounter this in the release.
(In reply to comment #15)
> That error would explain why content prefs don't work for you, since the
> TextZoom controller needs a reference to the content pref service to get and
> set prefs.  But I can't reproduce the problem here on Mac and Linux.

I'm seeing the problem on my mac now, zoom level just isn't getting remembered.
(In reply to comment #16)
> From browsing the file I guess it's because there's also left over text zoom
> prefs in there.  I'm attaching the file in case it's worth looking at.

Thanks!  I think this is actually a regression from the checkin for bug 403375, not a regression from this bug or a problem with the text zoom prefs (which shouldn't cause such problems, as the content pref datastore is designed to store arbitrary numbers and types of prefs).  Investigating...
(In reply to comment #18)
> (In reply to comment #16)
> > From browsing the file I guess it's because there's also left over text zoom
> > prefs in there.  I'm attaching the file in case it's worth looking at.
> 
> Thanks!  I think this is actually a regression from the checkin for bug 403375,
> not a regression from this bug or a problem with the text zoom prefs (which
> shouldn't cause such problems, as the content pref datastore is designed to
> store arbitrary numbers and types of prefs).  Investigating...

Yup, that's what it is.  Filed as bug 405683.
Summary: Web sites not centered, no scrollbars → When zooming: web sites not centered, no scrollbars
I can still cause this bug using a slightly more tricky set of steps, filed bug 406181
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: