Last Comment Bug 717143 - Remove bogus margins when force-reloading standalone image
: Remove bogus margins when force-reloading standalone image
Status: RESOLVED FIXED
[fixed-in-fx-team]
:
Product: Core
Classification: Components
Component: Layout: Images (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla13
Assigned To: Frank Yan (:fryn)
:
Mentors:
http://i.imgur.com/rJNOs.jpg
Depends on:
Blocks: 726402
  Show dependency treegraph
 
Reported: 2012-01-10 19:49 PST by :Felipe Gomes (needinfo me!)
Modified: 2013-11-13 02:20 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (1.73 KB, patch)
2012-02-10 15:51 PST, Frank Yan (:fryn)
bzbarsky: review+
felipc: feedback+
Details | Diff | Review

Description :Felipe Gomes (needinfo me!) 2012-01-10 19:49:40 PST
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) 2012-02-10 15:51:25 PST
Created attachment 596202 [details] [diff] [review]
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 me!) 2012-02-10 16:18:00 PST
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 [:bz] (Out June 25-July 6) 2012-02-17 22:05:44 PST
Comment on attachment 596202 [details] [diff] [review]
patch

r=me
Comment 4 Mozilla RelEng Bot 2012-02-17 23:26:39 PST
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 2012-02-18 03:31:06 PST
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/autolanduser@mozilla.com-7425b6a8bae5
Comment 6 Frank Yan (:fryn) 2012-02-18 03:41:42 PST
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) 2012-03-07 22:28:19 PST
Thanks for the review.

https://hg.mozilla.org/integration/fx-team/rev/d02afe4c49eb
Comment 8 Rob Campbell [:rc] (:robcee) 2012-03-08 06:48:27 PST
https://hg.mozilla.org/mozilla-central/rev/d02afe4c49eb

Note You need to log in before you can comment on or make changes to this bug.