Closed Bug 520943 Opened 10 years ago Closed 9 years ago

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

Categories

(Firefox :: Shell Integration, defect)

x86
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 4.0b7
Tracking Status
blocking2.0 --- -
status1.9.2 --- wontfix

People

(Reporter: fullmetaljacket.xp+bugmail, Assigned: robarnold)

References

Details

(Keywords: polish)

Attachments

(3 files, 2 obsolete files)

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).
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.
Assignee: nobody → tellrob
Status: NEW → ASSIGNED
Flags: blocking-firefox3.6?
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
This is how that tab actually looks ...
Flags: blocking-firefox3.6? → blocking-firefox3.6+
Unblocking, as we're not going to ship tab previews in Firefox 3.6 (see bug 525475).
Flags: blocking-firefox3.6+ → blocking-firefox3.6-
Duplicate of this bug: 525633
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.
Duplicate of this bug: 551925
this link shows it pretty well.
http://img.gottz.de/mfzoom.png
Blocks: 557733
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.
Attachment #448157 - Flags: review?(roc)
Attachment #448157 - Flags: review?(rflint)
Attachment #448157 - Flags: review?(rflint) → review?(gavin.sharp)
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.
(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.
Duplicate of this bug: 577199
blocking2.0: --- → ?
Duplicate of this bug: 578616
I don't think this is serious enough to block. Renominate if I'm missing an important issue.
blocking2.0: ? → -
Keywords: polish
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.
blocking2.0: - → ?
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.
Attachment #448157 - Attachment description: Pick the right scaling factor, snap the drawn areas to be pixel-aligned and allow for an epsilon in scaling when → Pick the right scaling factor, snap the drawn areas to be pixel-aligned and allow for an epsilon in scaling when
Attachment #448157 - Flags: review?(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.
Attachment #448157 - Flags: review?(dao) → review+
Attachment #448157 - Flags: review?(gavin.sharp)
Not blocking, would accept though.
blocking2.0: ? → -
Rob, are you working on a followup patch or is this ready for checkin?
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.
Attached patch Unbitrotted (obsolete) — Splinter Review
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.
Attachment #448157 - Attachment is obsolete: true
My apologies for the spam (first patch in a while). This one has the appropriate commit metadata.
Attachment #484517 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed http://hg.mozilla.org/mozilla-central/rev/afd457942f0d
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
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
thank you to rob and all moz dev who works on this bug.
Status: RESOLVED → VERIFIED
Target Milestone: Firefox 4.0b8 → Firefox 4.0b7
You need to log in before you can comment on or make changes to this bug.