520943 - Windows 7 taskbar previews does not respect browser's zoom level

Status: VERIFIED FIXED

(Firefox :: Shell Integration, defect)

Description

[-fullmetaljacket-]

Steps to Reproduce:
1. open atleast two tab then load your selected websites
2. set zoom level to 110% (goto View Menu > Zoom > Zoom In)

Actual Results:
large preview show page rendered with 100% level (and not with the zoom level set).

Comment 1

[Rob Arnold [:robarnold]]

I'm not entirely sure how to get the page zoom to appear. Vlad, do you know if drawWindow will handle zoomed pages correctly? I suppose I could figure out the zoom and set a scale transform on the thumbnail/preview if it doesn't.

Comment 2

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

Attachment: page preview with improper zoom

Sometimes it applies a completely wrong zoom factor which leads to a very broken look. For example, I normally view Bugzilla at -2 steps of zoom. Here's a screenshot of what the preview looks like

Comment 3

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

Attachment: for comparison, this is how it normally looks

This is how that tab actually looks ...

Comment 4

[Mike Shaver (:shaver emeritus)]

Unblocking, as we're not going to ship tab previews in Firefox 3.6 (see bug 525475).

Comment 6

[Rob Arnold [:robarnold]]

nsIPresShell::RenderDocument (the core of canvas's drawWindow and hence our tab preview implemenation) ignores the zoomlevel set on the pres shell. After a brief chat with roc on IRC, it should be possible to add a flag to RenderDocument (exposed to the canvas API of course) that takes the page zoom level into account when rendering.

Comment 8

[Jan-Stefan]

this link shows it pretty well.
http://img.gottz.de/mfzoom.png

Comment 9

[Rob Arnold [:robarnold]]

Attachment: Pick the right scaling factor, snap the drawn areas to be pixel-aligned and allow for an epsilon in scaling when

This turned out to be surprisingly tricky! Scaling by the document's zoom factor is incorrect as the actual scaling factor is usually slightly different. drawWindow sets a clip to the rectangle you're drawing and we need to make sure that it's pixel aligned in the device space or else native theming stops working because it can't get a region from the clip. We also need to allow for an epsilon difference in snapping pixels because small gaps in the rendered image will otherwise appear.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

What happened to the proposed solution in comment 6? It seems a lot nicer than having to worry about the scaling in the front end.

Comment 11

[Rob Arnold [:robarnold]]

(In reply to comment #10)
> What happened to the proposed solution in comment 6? It seems a lot nicer than
> having to worry about the scaling in the front end.

Truthfully I forgot about it. I looked into it again and it seems like I'd have to change a lot of code in awkward ways because nsPresShell::RenderDocument & PrepareContext assume that the gfxContext and related rectangles/regions are using the coordinate space of the of the window being drawn and not the canvas.

Comment 14

[Benjamin Smedberg]

I don't think this is serious enough to block. Renominate if I'm missing an important issue.

Comment 15

[Rob Arnold [:robarnold]]

This is an extremely noticeable bug whenever you have a zoomed page. It will either draw part of the page at a huge zoom or clip it at a normal zoom with the remainder of the tab content area painted solid black. The actual severity depends on the zoom level.

This bug makes the tab previews rather unhelpful. As I understand it, the Aero Peek integration is considered to be part of the primary UI for the browser so this is a major polish bug. Renominating to block 2.0.

Comment 16

[Rob Arnold [:robarnold]]

Comment on attachment 448157 [details] [diff] [review]
Pick the right scaling factor, snap the drawn areas to be pixel-aligned and allow for an epsilon in scaling when

Adding Dao for review in case he can get to it sooner than Gavin. Would really like to get this into the next beta.

Comment 17

[Dão Gottwald [:dao]]

Comment on attachment 448157 [details] [diff] [review]
Pick the right scaling factor, snap the drawn areas to be pixel-aligned and allow for an epsilon in scaling when

>+function snapRectAtScale(r, scale) {
>+  let x = Math.floor(r.x*scale);
>+  let y = Math.floor(r.y*scale);
>+  let width = Math.ceil((r.x+r.width)*scale) - x;
>+  let height = Math.ceil((r.y+r.height)*scale) - y;

add spaces before and after + and *

>+  XPCOMUtils.defineLazyGetter(this, "winutils",
>+    function () {
>+      let win = tab.linkedBrowser.contentWindow;
>+      return win.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
>+                .getInterface(Components.interfaces.nsIDOMWindowUtils);
>+  });

s/Components.interfaces/Ci/

I agree that front-end code should have to do this. Please file a follow-up bug on fixing this / providing the right APIs at some lower level.

Comment 18

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

Not blocking, would accept though.

Comment 19

[[not reading bugmail]]

Rob, are you working on a followup patch or is this ready for checkin?

Comment 20

[Rob Arnold [:robarnold]]

The gfx part has slight bitrot due to the gfx/ subtrees being rearranged. Not sure about the JS bits. I am not actively working on it due to more pressing commitments - this will miss beta6, possibly beta7 unless someone else picks this up.

Comment 21

[Rob Arnold [:robarnold]]

Attachment: Unbitrotted

Nits addressed and brought up to date. If bug 595842 were fixed, I do not think I would need to do the snapping since the native theming code would be able to handle non-integer coordinates in the clipping region.

Comment 22

[Rob Arnold [:robarnold]]

Attachment: Ready for checkin

My apologies for the spam (first patch in a while). This one has the appropriate commit metadata.

Comment 23

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Pushed http://hg.mozilla.org/mozilla-central/rev/afd457942f0d

Comment 24

[[not reading bugmail]]

Yeh!  No more black bars and a zoomed view!  Not bad at all, thanks Rob.
Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101019 Firefox/4.0b8pre ID:20101019165901

Comment 25

[-fullmetaljacket-]

thank you to rob and all moz dev who works on this bug.