475574 - iframe problem with rendering and image auto zoom

Status: RESOLVED DUPLICATE of bug 512367

(Core :: Layout: Images, Video, and HTML Frames, defect, P3)

Description

[Behnam Esfahbod [:zwnj]]

When there's an image in a iframe which is bigger than the iframe, auto zooming makes it smaller, which causes some render problems when changing the zoom, or scrolling the content of the iframe.

There might be problems with non-image formats, but I haven't notice any for HTML, nor PDF.

Going to attach the test-case.

Comment 1

[Behnam Esfahbod [:zwnj]]

Attachment: testcase: a simple iframe w/o width/height

Comment 2

[Behnam Esfahbod [:zwnj]]

Attachment: testcase: a simple iframe with width/height

Comment 3

[Behnam Esfahbod [:zwnj]]

Testcases show that if there are images with auto-zoom in iframes on sites with non-100% site-zoom, firefox gets confused on the images' zoom level.

How to reproduce:
- Open one of the testcases
- Change the zoom to something less that 100% (a few Ctrl+-)
- Test it: change the tab, click on the image to change the zoom level; change tab and come back, etc.

Comment 4

[Behnam Esfahbod [:zwnj]]

Attachment: screenshot of corupted image

Also there are render problems when scrolling the page too.

Comment 5

[Behnam Esfahbod [:zwnj]]

I forgot to mention that the lower narrower part of the image in that screenshot originally belonged to another image, in another iframe at the bottom of the page.

Comment 6

[Behnam Esfahbod [:zwnj]]

Also happens under Win XP.  The only difference there is that the mis-rendered part doesn't contain another images' data and is just black.

Comment 7

[(no longer active)]

Behnam: it would be best if you separate the two problems into two distinct bugs, and include a set of steps to reproduce the second problem.

Comment 8

[Behnam Esfahbod [:zwnj]]

(In reply to comment #7)
> Behnam: it would be best if you separate the two problems into two distinct
> bugs, and include a set of steps to reproduce the second problem.

I didn't to that because I think they have the same source.

Going to attach a better testcase.

Comment 9

[Behnam Esfahbod [:zwnj]]

Attachment: testcase: two frames with two big pictures from flickr.com

Comment 10

[Behnam Esfahbod [:zwnj]]

Just open the page and change the site-zoom to bigger than 100%, then click on the pictures to zoom and move around to see what happens.

Comment 11

[Behnam Esfahbod [:zwnj]]

Attachment: screenshot of testcase 3, on 120% site-zoom

Comment 12

[Behnam Esfahbod [:zwnj]]

Attachment: screenshot of testcase 3, on 100% site-zoom (no problem)

Comment 13

[(no longer active)]

(In reply to comment #10)
> Just open the page and change the site-zoom to bigger than 100%, then click on
> the pictures to zoom and move around to see what happens.

What I'm experiencing (in addition to the rendering problems) is that the image breaks the bounds of the containing iframe, which can potentially be used to cover parts of the parent frame.  I don't know if it can be used in an attack, but it's worth considering as a potential security problem.

I think any bug which can cause the contents of a frame to cover the contents of parent frames should be considered a blocker.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

While I agree that there's a potential security issue here, I don't think this blocks as the chain of social hacks and spoofing is pretty deeply chained.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

IMHO we should just turn off all the auto-zoom and user-zoom features for non-toplevel image documents. That shouldn't be too hard.

Comment 16

[(no longer active)]

(In reply to comment #15)
> IMHO we should just turn off all the auto-zoom and user-zoom features for
> non-toplevel image documents. That shouldn't be too hard.

IINM this should be done in nsImageDocument by detecting whether we are a top-level document, and set mResizeImageByDefault accordingly (to disable auto-zoom), and also adding event handlers in nsImageDocument::SetScriptGlobalObject only if we're top-level, thus disabling user-zoom.

The only thing which I couldn't figure out is how to detect whether we're a top-level document.  I'll have a patch if you can give me a pointer in that regard.  :-)

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm not sure of the best way to do that. You could just disable the behaviour when the docshell's parent (found via nsIDocShellTreeItem) is chrome, but I actually think that in an ideal world, Gecko would not have these behaviours by default, and Firefox would be responsible for implementing or at least enabling them in image documents in tabs. Gavin, what do you think?

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #17)
> I'm not sure of the best way to do that. You could just disable the behaviour
> when the docshell's parent (found via nsIDocShellTreeItem) is chrome,

Of course, I meant content.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Just a question: is a toplevel standalone image that's bigger than the browser window also broken on a site where the user has a non-identity preferred zoom for the site?

As far as enabling or disabling this, I think it makes the most sense for the behavior to live in Gecko and for apps to enable/disable via prefs (as we do now).  Checking for a toplevel same-type docshell makes sense to me if the problem is really limited to subframes, but see the first paragraph of this comment.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's not just because of this problem that we should disable the UI behaviour for subframes. I think if a page decides to use an <iframe>ed image, we shouldn't mess with it.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Yeah, fair enough.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It sounds like Ehsan can just go with the docshell parent type check then.

Comment 23

[Boris Zbarsky [:bzbarsky]]

I'd check whether the given docshell is its own same-type root, I think.

Comment 24

[(no longer active)]

(In reply to comment #23)
> I'd check whether the given docshell is its own same-type root, I think.

Is this different with comment 22?  By "same-type root" you mean that if the current docshell is content, the parent should be chrome and vice versa?  I think that would be wrong, because a chrome docshell can't have a content docshell as its parent (or can it?)

Comment 25

[Boris Zbarsky [:bzbarsky]]

> Is this different with comment 22? 

Subtly.  Instead of checking for specific types, you just check whether your docshell is the root docshell of its type.

> a chrome docshell can't have a content docshell as its parent

That's true, but do you really want to be assuming that only two docshell types exist and the exact relationship between them?

Comment 26

[(no longer active)]

(In reply to comment #25)
> > Is this different with comment 22? 
> 
> Subtly.  Instead of checking for specific types, you just check whether your
> docshell is the root docshell of its type.

I see.  Thanks for the explanation.

> > a chrome docshell can't have a content docshell as its parent
> 
> That's true, but do you really want to be assuming that only two docshell types
> exist and the exact relationship between them?

Probably not!  :-)

Comment 27

[Martijn Wargers (dead)]

Related to bug 495894?

Comment 28

[(no longer active)]

(In reply to comment #27)
> Related to bug 495894?

I think that bug is a dupe of this one.  I'll mark it as such.

Comment 30

[Timothy Nikkel (:tnikkel)]

It seems like this bug is fixed by bug 512367.

Comment 31

[Boris Zbarsky [:bzbarsky]]

Indeed.