Closed Bug 512367 Opened 12 years ago Closed 12 years ago

strange behaviour when user clicks to zoom an image in an iframe on a page that is being full paged zoomed

Categories

(Core :: Web Painting, defect, P2)

1.9.2 Branch
x86
All
defect

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta2-fixed

People

(Reporter: beltzner, Assigned: roc)

References

Details

(Keywords: regression)

Attachments

(2 files)

STR:

1. Visit https://bugzilla.mozilla.org/attachment.cgi?id=395281&action=edit
2. Click to zoom the screenshot that's in the attachment details frame

Expected: scrollbars at the edge of the iframe
Actual: scrollbars are inset, scrolling the zoomed image leaves artifacts (see screenshot)

Think this might be OSX only
Ah, this seems related to full-page zoom.

STR:
0. Visit bugzilla and hit cmd/ctrl+- a bit to zoom out the full page
1. Visit https://bugzilla.mozilla.org/attachment.cgi?id=395281&action=edit
2. Click to zoom the screenshot that's in the attachment details frame
OS: Mac OS X → All
Summary: strange behaviour when zooming an image in an iframe → strange behaviour when user clicks to zoom an image in an iframe on a page that is being full paged zoomed
Confirmed that this is a regression from Firefox 3; needs a full window, though. Maybe look around a month ago?
Related to bug 495894?
Might be related, but that got duped to a bug that was present in Firefox 3.5, and I can't reproduce this in Firefox 3.5.2.
Flags: blocking1.9.2?
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P2
Component: General → Layout: View Rendering
QA Contact: general → layout.view-rendering
OK, here's what I see:

2009-07-21-03 build (rev f2a58ffcd00c): Behavior as described in bug 475574 (weird zooming, scrollbars not invalidating right, but painting at the right place).

2009-07-22-03 build (rev 02f8bf10f441): Behavior as described in this bug.  Nice guess on "about a month ago".  ;)

Pushlog range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f2a58ffcd00c&tochange=02f8bf10f441

The obvious culprit in that range is bug 352093, which would just affect exactly which widget the bogus invalidates/paints are happening on, presumably...  The rest would be bug 475574.

I wonder whether this is just something that happens any time a page has a subframe with a different zoom level (so parts of the painting are using one zoom level and parts are using another one)...  That's exactly the situation with nsImageDocument, since it resets the zoom level.  When we apply site-specific zoom, do we just use the toplevel page setting and apply it to the , or can we set different frames to different values?  If the latter, this would give a non-imagedocument way of testing the above hypothesis.
Depends on: 475574
Right, the problem is likely that we assume appunits-per-dev-pixel is constant for all frames in a display list, but it is not. This assumption is implicit in everything that compares geometry across display list items. It's also implicit in nsIFrame::GetOffsetTo, for what it's worth...

This is not trivial to fix. Best thing to do here is probably to make nsSubdocumentFrame::BuildDisplayList detect when appunits-per-dev-pixel is different in the subdocument and in that case create a special display item which renders the subdocument as an atomic unit, much like nsDisplayTransform does.
That would probably be the robust fix, yeah...  We _might_ be able to fix this specific case by making nsImageDocument not mess with the zoom level if it's framed; that might be just as much work as the real fix, though, and won't help for extensions that zoom random subframes for some reason.
Assignee: nobody → roc
The real reason to do the fix in comment #7 is that when we unify all the frame trees, we'll need this fix to for full zoom of content windows to work.
Actually fixing it that way seems like a bad idea. We really do want to be able to inspect the display list geometry of zoomed subframes, so we can scroll with with bitblit scrolling and position and clip plugins in those subframes.

I think we may need to make all coordinates that are "relative to the reference frame" actually be scaled and converted correctly from the subframe to the coordinate system of the reference frame. That may be painful, I'll have to see how painful.
Attached patch fixSplinter Review
This fix just does what Boris suggested in comment #8: don't reset the zoom level if we're in a subframe and we toggle from zoomed-in to zoomed-out of vice versa. I think this is better behaviour, usually it's clear to the user that the containing document remains zoomed so there is no reason to unzoom the iframe.

The other thing I've done here is that in CheckOverflowing I don't want mVisibleWidth/mVisibleHeight to be altered to account for the zoom level, I'm leaving them as the true visible area. That's definitely what we want for subframes given the above change. I'm not sure what it's trying to achieve for top-level image documents. Tests pass anyway.
Attachment #405978 - Flags: review?(Olli.Pettay)
Whiteboard: [needs review]
Comment on attachment 405978 [details] [diff] [review]
fix

OK, works in the testcases I tried.
Attachment #405978 - Flags: review?(Olli.Pettay) → review+
Whiteboard: [needs review] → [needs landing]
Checked in:
http://hg.mozilla.org/mozilla-central/rev/af032ca9e9ec

This may have caused bug 522640, but probably not.
Flags: in-testsuite+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 192 landing]
No longer depends on: 475574
Duplicate of this bug: 475574
Depends on: 633768
Component: Layout: View Rendering → Layout: Web Painting
You need to log in before you can comment on or make changes to this bug.