Taskbar tab previews have clipped tab contents

RESOLVED WORKSFORME

Status

()

defect
P3
minor
RESOLVED WORKSFORME
9 years ago
9 years ago

People

(Reporter: gosimek, Assigned: robarnold)

Tracking

Trunk
Firefox 4.0
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

()

Attachments

(1 attachment, 2 obsolete attachments)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:2.0b1) Gecko/20100630 Firefox/4.0b1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:2.0b1) Gecko/20100630 Firefox/4.0b1

Tabs are below bookmarks. When FF 4.0b1 is opened and I move cursor to the taskbar FF icon the preview boxes appear - one for every tab. Now when I hover on one of them FF in background switches to this tab, but the site is 6px or 8px above the normal position and hover part of tabs.

Reproducible: Always

Steps to Reproduce:
1. Open FF
2. Set tab not "On the top"
3. Open at least 2 tab
4. Go to taskbar and hover tab preview
5. Look at FF in background



Screenshot: http://i27.tinypic.com/wwjjg3.png
Glitch also exist when tabs are on the top.
Component: Tabbed Browser → Widget: Win32
Product: Firefox → Core
QA Contact: tabbed.browser → win32
Blocks: 513162
Blocks: 474056
Component: Widget: Win32 → General
Product: Core → Firefox
Version: unspecified → Trunk
I can confirm I see this too on recent hourly.  The whole content area is shifting up on the Aero peek preview, scrollbars and all.
Might not be related toi the new titlebar, I don't see any relationship between the black margins and the non client margins. cc'ing roc, might be due to retained layers.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Odd, this only shows up on certain sites, bugzilla happens to be one of them.
Posted image tab/live previews anomoly (obsolete) —
Blocks: 564991
Ahh, nope nope nope. I had bugzilla zoom set. :) I think this is the tab preview zoom problem.
No longer blocks: 513162, 564991
(In reply to comment #6)
> Ahh, nope nope nope. I had bugzilla zoom set. :) I think this is the tab
> preview zoom problem.

Looks like it to me.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 520943
Are you shure this is a duplicate? The original reportet problem (content shifted upwards) happens to me without any zoom set (4.0b1). In addition, Jim's screenshot seems to show a completely unrelated problem.
It's not an duplicate...
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(In reply to comment #8)
> Are you shure this is a duplicate? The original reportet problem (content
> shifted upwards) happens to me without any zoom set (4.0b1). In addition, Jim's
> screenshot seems to show a completely unrelated problem.

I can't reproduce without zoom on a particular site set. Thumbnails display correctly. Can you screenshot a set of previews maybe for your case?
(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US;
> rv:2.0b1) Gecko/20100630 Firefox/4.0b1
> Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US;
> rv:2.0b1) Gecko/20100630 Firefox/4.0b1
> 
> Tabs are below bookmarks. When FF 4.0b1 is opened and I move cursor to the
> taskbar FF icon the preview boxes appear - one for every tab. Now when I hover
> on one of them FF in background switches to this tab, but the site is 6px or
> 8px above the normal position and hover part of tabs.
> 
> Reproducible: Always
> 
> Steps to Reproduce:
> 1. Open FF
> 2. Set tab not "On the top"
> 3. Open at least 2 tab
> 4. Go to taskbar and hover tab preview
> 5. Look at FF in background
> 
> 
> 
> Screenshot: http://i27.tinypic.com/wwjjg3.png

Doh, never mind. So could you try a nightly for this? This might be resolved fixed from another bug.
Attachment #458002 - Attachment is obsolete: true
Blocks: 513162
No longer blocks: 474056
> Doh, never mind. So could you try a nightly for this? This might be resolved
> fixed from another bug.

Still present in Mozilla/5.0 (Windows; Windows NT 6.1; Win64; x64; rv:2.0b2pre) Gecko/20100720 Minefield/4.0b2pre
(In reply to comment #12)
> > Doh, never mind. So could you try a nightly for this? This might be resolved
> > fixed from another bug.
> 
> Still present in Mozilla/5.0 (Windows; Windows NT 6.1; Win64; x64; rv:2.0b2pre)
> Gecko/20100720 Minefield/4.0b2pre

and the key problem is that w/tabs on bottom, tabs get clipped by the page content? (Which is highlighted in your screenshot.)
Ahh, confirmed on a nightly.

- maximized window
- live previews have clipped tabs
- the browser is fine
Severity: normal → minor
blocking2.0: --- → ?
Priority: -- → P3
Target Milestone: --- → Firefox 4.0
Summary: Glitch with Windows 7 taskbar preview windows and tabs in FF → Taskbar tab previews have clipped tab contents
(In reply to comment #14)
> Ahh, confirmed on a nightly.
> 
> - maximized window
> - live previews have clipped tabs
> - the browser is fine

Yup, also see comment 2 for additional info.
Did some minimal debugging today. When unmaximized, we draw the tab contents 2px too high. When maximized, we draw them 8px too high. Jim, I think this is due to your nonclient margins change. It seems that the logical origin of the browser chrome is no longer the same as the image origin.
(In reply to comment #16)
> Did some minimal debugging today. When unmaximized, we draw the tab contents
> 2px too high. When maximized, we draw them 8px too high. Jim, I think this is
> due to your nonclient margins change. It seems that the logical origin of the
> browser chrome is no longer the same as the image origin.

The origin of the window is -8x-8 on the screen when maximized. With non-client margins, the content sits at the origin. Any idea where in the code we calculate the tab location for drawing?
(In reply to comment #17)
> (In reply to comment #16)
> > Did some minimal debugging today. When unmaximized, we draw the tab contents
> > 2px too high. When maximized, we draw them 8px too high. Jim, I think this is
> > due to your nonclient margins change. It seems that the logical origin of the
> > browser chrome is no longer the same as the image origin.
> 
> The origin of the window is -8x-8 on the screen when maximized. With non-client
> margins, the content sits at the origin. Any idea where in the code we
> calculate the tab location for drawing?

Looks like it might be here - 

    ctx.drawWindow(this.win.win, 0, 0, width, height, "transparent");

    // Compositor, where art thou?
    // Draw the tab content on top of the toplevel window
    this.updateCanvasPreview();

    let boxObject = this.linkedBrowser.boxObject;
    ctx.translate(boxObject.x, boxObject.y);
    ctx.drawImage(this.canvasPreview, 0, 0);

Might the box object values be off kilter from the 0,0 origin of the drawWindow call, since the y origin of the window is -8?
(In reply to comment #18)
> Might the box object values be off kilter from the 0,0 origin of the drawWindow
> call, since the y origin of the window is -8?

When not maximized, the origin appears to be -2. I took screenshots of both states and the values of boxObject indicate that it expects the origin's y coordinate to be the row with the first orange pixel.
(In reply to comment #19)
> When not maximized, the origin appears to be -2. I took screenshots of both
> states and the values of boxObject indicate that it expects the origin's y
> coordinate to be the row with the first orange pixel.

In simpler terms, the origin for boxObject does not take the margin into account. This is our problem - we now have a XUL <window> with non-zero margin.
Posted patch Slight hack v1 (obsolete) — Splinter Review
So I can take the computed style for the margin properties and use that but this solution feels a bit hackish to me. I am wondering if there is a better way to determine the actual offset (i.e. some property I've overlooked).
Assignee: nobody → tellrob
Status: REOPENED → ASSIGNED
Attachment #460442 - Flags: feedback?(roc)
Attachment #460442 - Flags: feedback?(gavin.sharp)
Attachment #460442 - Flags: feedback?(dao)
Comment on attachment 460442 [details] [diff] [review]
Slight hack v1

>diff --git a/browser/components/wintaskbar/WindowsPreviewPerTab.jsm b/browser/components/wintaskbar/WindowsPreviewPerTab.jsm

>   previewTabCallback: function (ctx) {

>+    let elem = domWin.document.getElementById('main-window');

document.documentElement

>+    function extractPixelProperty(prop) {
>+      let prop = domWin.getComputedStyle(elem,null).getPropertyValue(prop);
>+      return parseInt(prop.slice(0,-2));

No need to slice, parseInt will ignore the trailing "px".

>+    let x = boxObject.x + extractPixelProperty('margin-left');
>+    let y = boxObject.y + extractPixelProperty('margin-top');
>+    ctx.translate(x, y);

The x case isn't actually needed, is it? We don't set horizontal margins on the window, and I'm not sure there would ever be a reason to. (If we did, would it be -moz-margin-start? Probably not, I guess...)
Attachment #460442 - Flags: feedback?(gavin.sharp) → feedback+
(In reply to comment #19)
> (In reply to comment #18)
> > Might the box object values be off kilter from the 0,0 origin of the drawWindow
> > call, since the y origin of the window is -8?
> 
> When not maximized, the origin appears to be -2. I took screenshots of both
> states and the values of boxObject indicate that it expects the origin's y
> coordinate to be the row with the first orange pixel.

There's some css that sets margin-top to 8px in this case.

http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser/browser-aero.css (In reply to comment #22)

> Comment on attachment 460442 [details] [diff] [review]
> The x case isn't actually needed, is it? We don't set horizontal margins on the
> window, and I'm not sure there would ever be a reason to. (If we did, would it
> be -moz-margin-start? Probably not, I guess...)

I think it will be once we implement our own window frame. Content will be sitting at 8x8 in the client area when in the maximized state.
Noticed the two pixel offset is in there as well. (Not sure when that got added.) 

I think these will need to be updated when the custom frame is implemented, since the frame border width & height will be based on the theming code that hasn't landed yet, and will equal whatever the current user has set for border widths through the advanced window settings properties.

Maybe we should mark this dependent on the framing (bug 575870)?
No longer blocks: 513162
Depends on: 575870
I went ahead and changed dependencies. Once we have an actual frame tied to system display prefs, this should be pretty easy to fix up.
Unless we're going to fix that soon, I'd rather get this in for b3.
Attachment #460442 - Attachment is obsolete: true
Attachment #461063 - Flags: review?(gavin.sharp)
Attachment #460442 - Flags: feedback?(roc)
Attachment #460442 - Flags: feedback?(dao)
Comment on attachment 461063 [details] [diff] [review]
Slight hack v2

>diff --git a/browser/components/wintaskbar/WindowsPreviewPerTab.jsm b/browser/components/wintaskbar/WindowsPreviewPerTab.jsm

>   previewTabCallback: function (ctx) {

>+    // domWin is not an actual DOM element

I don't understand the purpose of this comment. Just remove it?

>+    let elem = domWin.document.documentElement;
>+    let style = domWin.getComputedStyle(elem,null);
>+    function extractPixelProperty(prop) {
>+      let prop = style.getPropertyValue(prop);
>+      return parseInt(prop);
>+    }
>+    let x = boxObject.x + extractPixelProperty('-moz-margin-start');
>+    let y = boxObject.y + extractPixelProperty('margin-top');
>+    ctx.translate(x, y);

I don't understand how Jim proposes we fix this after bug 575870 lands, but it sounds like it would mean removing this hack? r=me assuming that's true, and we get a followup filed on removing this.
Attachment #461063 - Flags: review?(gavin.sharp) → review+
Blocking+ for janky-looking primary UI.
blocking2.0: ? → final+
Duplicate of this bug: 582206
QA Contact: win32 → general
Should be fixed by the landings in bug 575870 & bug 574454.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
looks like this got fixed at some point. resolving wfm.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.