Remove bogus margins when force-reloading standalone image

RESOLVED FIXED in mozilla13

Status

()

Core
Layout: Images
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Felipe, Assigned: fryn)

Tracking

Trunk
mozilla13
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-fx-team], URL)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Assignee: nobody → fryn
Status: NEW → ASSIGNED
OS: Windows 7 → All
Hardware: x86_64 → All
Summary: Force reloading a standalone image adds margins to the page → Remove bogus margins when force-reloading standalone image
Version: unspecified → Trunk
(Assignee)

Comment 1

5 years ago
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?
Attachment #596202 - Flags: review?(bzbarsky)
Attachment #596202 - Flags: feedback?(felipc)
(Reporter)

Comment 2

5 years ago
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!
Attachment #596202 - Flags: feedback?(felipc) → feedback+
Comment on attachment 596202 [details] [diff] [review]
patch

r=me
Attachment #596202 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

5 years ago
Whiteboard: [autoland]

Updated

5 years ago
Whiteboard: [autoland] → [autoland-in-queue]

Comment 4

5 years ago
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

5 years ago
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

Updated

5 years ago
Whiteboard: [autoland-in-queue]
(Assignee)

Updated

5 years ago
Blocks: 726402
(Assignee)

Comment 6

5 years ago
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.
(Assignee)

Comment 7

5 years ago
Thanks for the review.

https://hg.mozilla.org/integration/fx-team/rev/d02afe4c49eb
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d02afe4c49eb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.