717143 - Remove bogus margins when force-reloading standalone image

Status: RESOLVED FIXED

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

Description

[:Felipe Gomes (needinfo for replies!)]

When viewing a standalone image that was shrunk to fit the screen, a force-reload of the page reloads the image but adds margins to it.

STR:
- open an image that is larger than the window. e.g. http://i.imgur.com/rJNOs.jpg
- notice that the image is connected to the top/bottom edge of the content area
- Press Ctrl+Shift+R
- notice that now there's an 8px margin on the image

Any resizing of the window brings it back to normal. DOM Inspector tells everything has 0 on margins, but displays 8px for the top/bottom computed style of the <img>, despite being nothing in the stylesheets setting that.

Comment 1

[Frank Yan (:fryn)]

Attachment: patch

The problem is that CheckOverflowing() is deflating the visibleArea by the sizes of the <body/>'s margins, borders, and padding, but the values that it gets for the margin are erroneously non-zero when force-reloading an image, causing this bug. We explicitly apply zero margins to the <body/> for ImageDocuments anyway, so I removed that unnecessary code.

Do we still need the extra block (scope) and the comment at the top of it?

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 596202 [details] [diff] [review]
patch

This fixes the problem.

It's weird that mCachedMargins has the wrong default margins after a force-refresh, but I guess it's expected as the stylesheet hasn't been loaded at that point.

anyway, it seems like removing code that is doing unecessary work and producing the wrong results is a good compromise!

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 596202 [details] [diff] [review]
patch

r=me

Comment 4

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 596202
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=7425b6a8bae5
Try run started, revision 7425b6a8bae5. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=7425b6a8bae5

Comment 5

[Mozilla RelEng Bot]

Try run for 7425b6a8bae5 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=7425b6a8bae5
Results (out of 212 total builds):
    success: 166
    warnings: 32
    failure: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-7425b6a8bae5

Comment 6

[Frank Yan (:fryn)]

We need to land this at the same time as my fix for bug 726402 (a random orange related to this), so wait for that.

Comment 7

[Frank Yan (:fryn)]

Thanks for the review.

https://hg.mozilla.org/integration/fx-team/rev/d02afe4c49eb

Comment 8

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/d02afe4c49eb