Broken windows animation in Vista Aero with 3.1 Alpha and nightly

VERIFIED FIXED

Status

()

P1
normal
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: magicrisco, Assigned: jimm)

Tracking

({fixed1.9.1, polish, regression})

Trunk
x86
Windows Vista
fixed1.9.1, polish, regression
Points:
---
Bug Flags:
blocking1.9.1 -
wanted1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [polish-hard][polish-interactive][polish-p1])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-GB; rv:1.9.0.1) Gecko/2008070208 Firefox
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-GB; rv:1.9.0.1) Gecko/2008070208 Firefox

Animation in for windows in the 3.1 alpha and windows nightly is broken. When opening firefox or any windows they appear instant, when they should fade in to view.

This works fine in 3.01, but in 3.1 the fade in animation is broken. It only animates on close. This affects the opening of the main firefox window, and any subsequent ones such as options /bookmarks / new windows etc.

I will attach two videos to show the problem.

Reproducible: Always

Steps to Reproduce:
1. Load up Firefox 3.1
2. Open options

Actual Results:  
Windows do not fade in.

Expected Results:  
Windows should fade in.
(Reporter)

Comment 1

10 years ago
Here is a video with 3.01 animating the windows

http://www.uploading.com/files/J67SGR0R/3.01.avi.html
(Reporter)

Comment 2

10 years ago
Here is a video of 3.1 NOT animating the windows

http://www.uploading.com/files/UJGRVS9T/3.1.avi.html
(Reporter)

Updated

10 years ago
Flags: blocking-firefox3.1?
Version: unspecified → Trunk
(Reporter)

Comment 3

10 years ago
Got a regression range:

Works:

07.08.08 http://forums.mozillazine.org/viewtopic.php?f=23&t=784265

Fails

08.08.08 http://forums.mozillazine.org/viewtopic.php?f=23&t=785975
(Reporter)

Updated

10 years ago
Keywords: regression, testcase

Updated

10 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Updated

10 years ago
Severity: major → blocker

Comment 4

10 years ago
CONFIRMED as described above!

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a2pre) Gecko/20080817033619 Minefield/3.1a2pre 

Comment 5

10 years ago
Karl, it is definitely is not a blocker, critical or a major bug.  It is a very minor bug.
Severity: blocker → minor
Flags: blocking-firefox3.1? → wanted-firefox3.1?

Updated

10 years ago
Keywords: testcase
Component: OS Integration → Widget: Win32
Flags: wanted-firefox3.1?
Product: Firefox → Core
QA Contact: os.integration → win32
(Reporter)

Comment 6

10 years ago
Is there going to be any movement on this? Since it was busted down to minor, nobody is taking notice of it. With the first beta not far off, would it not be wise to implement a fix?
I looked at it. There definitely is a regression between fx3 and trunk, but I didn't see anything in the regression range that looked like it was the cause, but I'm not too familiar with why the windows choose to animate or not.

Updated

10 years ago
Severity: minor → normal
Keywords: qawanted
(Reporter)

Comment 8

10 years ago
Well still broken, and no sign of anyone attempting to fix it? How would I go about getting more exposure to this bug?
Can you narrow it down to the particular changeset the broke it?

Comment 10

10 years ago
(In reply to comment #8)
> Well still broken, and no sign of anyone attempting to fix it? How would I go
> about getting more exposure to this bug?
Annoying the heck out of people isn't going to encourage them to fix it.  Someone will get around to it one day when they are bored and want to fix something that means absolulety nothing.  It is such a trivial issue that there is absoluetly no point in you bugspamming.

Comment 11

10 years ago
Works: 7895ffd97cb2
Broken: ca7bb3f59be1

So bug 438987 somehow caused this...
Blocks: 438987
Keywords: qawanted
Wanted request had not been carried forward when component changed form Firefox to Core.
Flags: wanted1.9.1?
I remember some odd behavior I observed while debugging for bug 451300. The window will be change to transparent and then back to opaque. This used to not be the case. I wonder if making a window transparent causes it to not be animated by the DWM.
This is going to end up being a polish issue, so preemptively marking it P1/wanted.  Needs analysis of the changes in bug 348987...
Flags: wanted1.9.1?
Flags: wanted1.9.1+
Flags: blocking1.9.1-
Priority: -- → P1

Updated

10 years ago
Whiteboard: [polish-easy][polish-interactive]

Updated

10 years ago
Keywords: polish
Whiteboard: [polish-easy][polish-interactive] → [polish-hard][polish-interactive]
Whiteboard: [polish-hard][polish-interactive] → [polish-hard][polish-interactive][polish-high-visibility]
(Reporter)

Comment 15

10 years ago
Is there a chance of this being fixed, nobody has looked at since November.
(Assignee)

Updated

10 years ago
Assignee: nobody → jmathies
(Assignee)

Comment 16

10 years ago
Ok, I've been able to reproduce this. It is a pretty ugly polish issue, not sure why we didn't block on it. Starting in on it today... lets hope there's an easy fix.
(Assignee)

Comment 17

10 years ago
Created attachment 360393 [details] [diff] [review]
frame transparency fix v.1
(Assignee)

Updated

10 years ago
Attachment #360393 - Flags: review?(roc)
(Assignee)

Comment 18

10 years ago
Comment on attachment 360393 [details] [diff] [review]
frame transparency fix v.1

A slight touch up from the patch that landed in bug 438987. We snipped out some code that was needed.

Updated

10 years ago
Component: Widget: Win32 → Layout
QA Contact: win32 → layout
This seems like a bit of a hack. There's no real reason to not treat a viewportFrame as a canvas. Can we address this more reasonably in GetFrameTransparency?
(Assignee)

Comment 20

10 years ago
(In reply to comment #19)
> This seems like a bit of a hack. There's no real reason to not treat a
> viewportFrame as a canvas. Can we address this more reasonably in
> GetFrameTransparency?

I'll take a look.
(Assignee)

Comment 21

10 years ago
Created attachment 360544 [details] [diff] [review]
frame transparency fix v.2
Attachment #360393 - Attachment is obsolete: true
Attachment #360393 - Flags: review?(roc)
(Assignee)

Comment 22

10 years ago
Comment on attachment 360544 [details] [diff] [review]
frame transparency fix v.2

The ifdef'ing could probably be left out since code similar to this was in cross platform code for a number of years. But I figured isolating it to win couldn't hurt since it's not needed on any other platform.

The other thing I was wondering about was if there is a way to detect a newly created frame from through nsIFrame's properties rather than using the first child. I looked around and didn't find anything. First child does works well though and was used this way prior to bug 438987 landing.
Attachment #360544 - Flags: review?(roc)
Let's not use #ifdefs. Cross platform consistency is important.

So when the window changes from transparent to opaque, are we actually notifying the widget, and is that notifying Vista?
I mean before this patch.

I guess I'm trying to understand if we're working around a Vista bug here.
(Assignee)

Comment 25

10 years ago
(In reply to comment #23)
> Let's not use #ifdefs. Cross platform consistency is important.

No problem, I'll take it out.

> So when the window changes from transparent to opaque, are we actually
> notifying the widget, and is that notifying Vista?

Previously IsCanvasFrame would return false as long as the view had no children. In that case, nsContainerFrame's SyncFrameViewGeometryDependentProperties quit processing before setting transparency on the window. I'm guessing the default was opaque.

After IsCanvasFrame started returning true for an empty view, the transparency was set to transparent through calls to nsLayoutUtils::GetFrameTransparency and widget->SetTransparencyMode(mode) -

http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsContainerFrame.cpp#485

that caused a call into nsWindow that added WS_EX_LAYERED extended style, which killed the fade effect vista uses on conventional windows. Once the view's content filled in, the transparency was set back to opaque and the window popped into view.

The change I made returns opaque when there are no children (a good test for a newly created window?) so that the layering never gets turned on, and the fade effect takes place.

I'm not sure I'd characterize this as a vista bug, skipping off the fade on layered windows is probably by design.
But we remove the WS_EX_LAYERED bit *before* the fade effect is triggered, right? Or is this a fade-in effect?
(Assignee)

Comment 27

10 years ago
(In reply to comment #26)
> But we remove the WS_EX_LAYERED bit *before* the fade effect is triggered,
> right? Or is this a fade-in effect?

It's a fade-in effect on windows we create like the main window and dialogs for things like preferences. The first time we set transparency to transparent occurs on the very first call into nsXULDocument::StartLayout, and we set it back to opaque just a short while later. This patch prevents that. The set in StartLayout seems to be enough for vista to give up doing a fade-in effect and instead just display the complete window. That results in a sort of old school 'pop' when the window displays. On older operating systems there wouldn't be a difference, but on vista it's pretty obvious.
Attachment #360544 - Flags: superreview+
Attachment #360544 - Flags: review?(roc)
Attachment #360544 - Flags: review+
Comment on attachment 360544 [details] [diff] [review]
frame transparency fix v.2

Alright, let's do this. The comment should explain that we need an uninitialized window to be treated as opaque because doing otherwise confuses some platforms, specifically Vista.
(Assignee)

Comment 29

10 years ago
Created attachment 360638 [details] [diff] [review]
landed patch
Attachment #360544 - Attachment is obsolete: true
(Assignee)

Comment 30

10 years ago
http://hg.mozilla.org/mozilla-central/rev/64bebcc65154
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Attachment #360638 - Flags: approval1.9.1?
(Reporter)

Comment 31

10 years ago
Verfied working and fixed on Vista X64 Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090205 Minefield/3.2a1pre. Thanks Jim!
Status: RESOLVED → VERIFIED
Attachment #360638 - Flags: approval1.9.1? → approval1.9.1+
Adding the fixed1.9.1 keyword to this bug so that this can be tracked.
Whiteboard: [polish-hard][polish-interactive][polish-high-visibility] → [polish-hard][polish-interactive][polish-high-visibility], fixed1.9.1

Updated

10 years ago
Keywords: fixed1.9.1
Whiteboard: [polish-hard][polish-interactive][polish-high-visibility], fixed1.9.1 → [polish-hard][polish-interactive][polish-high-visibility]
This bug's priority relative to the set of other polish bugs is:
P1 - Polish issue that appears in the main window, or is something that the user may encounter several times a day.

This was a significant issue with opening and closing the main window.
Whiteboard: [polish-hard][polish-interactive][polish-high-visibility] → [polish-hard][polish-interactive][polish-p1]
You need to log in before you can comment on or make changes to this bug.