Frame border doesn't repaint properly when the main window loses focus

RESOLVED FIXED

Status

()

Core
Widget: Win32
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Stanimir Stamenkov, Assigned: jimm)

Tracking

(Blocks: 1 bug)

unspecified
x86
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 final+)

Details

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.24) Gecko/20100228 SeaMonkey/1.1.19 (Spidey; Mnenhy 0.7.6)
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre

The main window frame doesn't get repainted properly to reflect inactive status when the window looses focus.  This appears to happen when switching between Minefield windows.  If I activate the main Minefield window then click on the desktop - the main window frame gets repainted o.k.

If I activate the main Minefield window then open or activate existing "Organize Bookmarks" window only the title bar of the main window gets repainted to inactive state - the side and bottom border remain painted as active.

Happens with both XP style and Classic themes.  With Classic themes one notices a secondary issue (the top border not being painted at all), but I'll fill a separate bug for it.  For seeing the problem with Classic themes explicitly set the "Active Window Border" and "Inactive Window Border" colors to something different than the default and distinguishable (like bright yellow and cyan).


Reproducible: Always




This appears with the introduction of title-bar drawn appmenu-button on WinXP.
(Reporter)

Comment 1

7 years ago
Created attachment 464754 [details]
XP-style inactive main window shows side/bottom border as active
(Reporter)

Comment 2

7 years ago
Created attachment 464756 [details]
XP-style active main window - secondary window frame border rendered o.k.
(Reporter)

Comment 3

7 years ago
Created attachment 464758 [details]
Classic theme window border color settings
(Reporter)

Comment 4

7 years ago
Created attachment 464762 [details]
Classic theme main window shows side/bottom border as active
(Reporter)

Comment 5

7 years ago
Created attachment 464763 [details]
Classic theme active main window - secondary window frame border rendered o.k.
(Reporter)

Comment 6

7 years ago
This appears to happen only with browser windows which have the app-menu button shown.  When I permanently show the menu bar and the app-menu button disappears the window border starts to paint all right in the case given.

Updated

7 years ago
Component: Theme → Widget: Win32
Product: Firefox → Core
QA Contact: theme → win32
(Assignee)

Updated

7 years ago
Depends on: 575870
(Assignee)

Updated

7 years ago
Blocks: 574454
No longer depends on: 575870
(Assignee)

Comment 7

7 years ago
confirmed. There's something going on with wm_ncactivate where windows normally caches the active state. When we handle this event internally, the cache isn't set. This only happens if the window involved is from the same process. Switching to other application windows doesn't exhibit the problem. We may have to paint the border ourselves if we can't trick windows into painting the right frame.

This should block the 4.0 release.
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
blocking2.0: ? → final+
(Assignee)

Updated

7 years ago
Assignee: nobody → jmathies
(Assignee)

Updated

7 years ago
Summary: Frame border doesn't repaint properly when the main window looses focus → Frame border doesn't repaint properly when the main window loses focus
(Assignee)

Comment 8

7 years ago
I've managed to pin down the cause of this although I'm still looking for a simple solution. We intercept WM_NCACTIVATE and handle that message internally to prevent the theme library from painting to the titlebar. Unfortunately this causes the caption state to get out of sync on the window. The theme library calls GetWindowInfo in deciding what theme to use, this returns the wrong value and the library paints the wrong borders.

Worst case scenario I can hook GetWindowInfo, although I'd like to try and get windows to sync up it's internal info correctly first.
(Assignee)

Comment 9

7 years ago
Created attachment 489413 [details] [diff] [review]
track caption status internally v.1

Will kick off some reviews after a little bake time in my patch queue.
(Assignee)

Comment 10

7 years ago
Created attachment 489414 [details] [diff] [review]
track caption status internally v.1

- removed some debug junk
Attachment #489413 - Attachment is obsolete: true
(Assignee)

Comment 11

7 years ago
Created attachment 489953 [details] [diff] [review]
track caption status internally v.2

The theme library uxtheme.dll uses GetWindowInfo in it's ncpaint handler. The caption status isn't updated unless def wnd proc handles wm_ncactivate, which we currently handle internally to prevent windows from painting over our titlebar. So this just hook into GetWindowInfo so we can update the caption status we track when the theme library makes the call.
Attachment #489414 - Attachment is obsolete: true
Attachment #489953 - Flags: review?(vladimir)
(Assignee)

Updated

7 years ago
Attachment #464762 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #464758 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #464763 - Attachment is obsolete: true
Comment on attachment 489953 [details] [diff] [review]
track caption status internally v.2

the amount of hooking we're doing is terrifying :p
Attachment #489953 - Flags: review?(vladimir) → review+
(Assignee)

Comment 13

7 years ago
http://hg.mozilla.org/mozilla-central/rev/59bbc730aee4
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
I checked in a bustage fix for non-IPC builds for this:

http://hg.mozilla.org/mozilla-central/rev/ef89316c2b33

RemoveProp needed to be changed to RemovePropW as that's what the argument passed to it is using. IPC builds tend to turn off -pedantic in various places which is why this wasn't picked up on FF.
(In reply to comment #14)
> I checked in a bustage fix for non-IPC builds for this:
> 
> http://hg.mozilla.org/mozilla-central/rev/ef89316c2b33
> 
> RemoveProp needed to be changed to RemovePropW as that's what the argument
> passed to it is using. IPC builds tend to turn off -pedantic in various places
> which is why this wasn't picked up on FF.

Don't IPC builds tend to turn on _UNICODE too?
(Assignee)

Comment 16

7 years ago
(In reply to comment #14)
> I checked in a bustage fix for non-IPC builds for this:
> 
> http://hg.mozilla.org/mozilla-central/rev/ef89316c2b33
> 
> RemoveProp needed to be changed to RemovePropW as that's what the argument
> passed to it is using. IPC builds tend to turn off -pedantic in various places
> which is why this wasn't picked up on FF.


That is odd. I'd understand a broken sea monkey build, but not a non-MOZ_IPC build.

Anyway, sorry about the bustage and thanks for the fix.
(Reporter)

Comment 17

7 years ago
(In reply to comment #13)
> http://hg.mozilla.org/mozilla-central/rev/59bbc730aee4

Is the fix supposed be in the latest nightlies?
(Assignee)

Comment 18

7 years ago
(In reply to comment #17)
> (In reply to comment #13)
> > http://hg.mozilla.org/mozilla-central/rev/59bbc730aee4
> 
> Is the fix supposed be in the latest nightlies?

yep!
(Reporter)

Comment 19

7 years ago
It seems it only fixes the XP theme case.  It still doesn't work for me using a Classic theme on both WinXP and Win7.
(Assignee)

Comment 20

7 years ago
(In reply to comment #19)
> It seems it only fixes the XP theme case.  It still doesn't work for me using a
> Classic theme on both WinXP and Win7.

Ah yeah, this fix is for themed desktops only. Let's file a follow up, I'll take a look when I get back to bug 586233.
(Reporter)

Comment 21

7 years ago
Is there a follow up already filed, or I need to file one?

Comment 22

7 years ago
(In reply to comment #19)
> It seems it only fixes the XP theme case.  It still doesn't work for me using a
> Classic theme on both WinXP and Win7.

Still broken in 4.0b11; should probably reopen this or file a new bug?

Comment 23

7 years ago
I see this is mainly talking about the window borders, but I'm seeing the title bar color not properly changing (blue/gray) when the focus changes, using the Classic theme on XP.  Apologies if there is a different bug for that...
(Assignee)

Comment 24

7 years ago
(In reply to comment #23)
> I see this is mainly talking about the window borders, but I'm seeing the title
> bar color not properly changing (blue/gray) when the focus changes, using the
> Classic theme on XP.  Apologies if there is a different bug for that...

bug 622542 probably. not sure if that got into b11.
(Assignee)

Updated

7 years ago
Blocks: 636264
You need to log in before you can comment on or make changes to this bug.