622542 - Switching main window between active and inactive doesn't update the title bar active state

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[horvathd]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b9pre) Gecko/20110103 Firefox/4.0b9pre
Build Identifier: 20110103030359

If I hide the menu bar, and I open an another program with a non maximized window then the title bar of Firefox doesn't change to inactive automatically, only if I move the cursor over the title bar. Also when I click back to Firefox then it doesn't change back to active.

Reproducible: Always

Steps to Reproduce:
1. Open firefox
2. Open another software with a non maximized window.
3. Make Firefox the active window
4. Click on the other program on the taskbar to make it active
Actual Results:  
Firefox' title bar doesn't change to inactive

Expected Results:  
It should change to inactive

But there are exceptions when the change is working.

1. Text is selected in Firefox,
2. The cursor is in a textbox in Firefox.

Comment 1

[Jim Mathies [:jimm]]

What windows theme are you using?

Comment 2

[horvathd]

The default Luna theme.

I checked with Firefox beta 8 and using this there is no problem.

Comment 3

[Jim Mathies [:jimm]]

confirmed, this should block.

Comment 4

[Jim Mathies [:jimm]]

Can anyone else confirm this goes away after the browser sits open for about a minute?

Comment 5

[Jim Mathies [:jimm]]

Ah, never mind. If you have the cursor focus in the search edit, the problem goes away. Clearly some sort of invalidation/repainting problem.

Comment 6

[Jim Mathies [:jimm]]

Regression range - 12/27 -> 12/28:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=24b63f638579&tochange=e928817fb4e9

Comment 7

[Jim Mathies [:jimm]]

(In reply to comment #6)
> Regression range - 12/27 -> 12/28:
> 
> http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=24b63f638579&tochange=e928817fb4e9

Confirmed, backing out part 2 from bug 615870 fixes this - Part 2: Track per-display-root-frame 'update layer tree' state bit. r=tnikkel a=b-f

Comment 8

[Benjamin Smedberg]

tn, can you take this?

Comment 9

[Timothy Nikkel (:tnikkel)]

Any time we get a paint event that wasn't preceded by an nsIFrame::Invalidate call we will try to do an empty transaction. I bet Windows sends us a paint event when we become inactive?

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I bet PresShell::DocumentStatesChanged needs to mark the layer tree as needing an update.

Or maybe FrameLayerBuilder::InvalidateAllLayers/InvalidateAllThebesLayerContents/InvalidateThebesLayersInSubtree should all ensure the layer tree gets updated at the next paint.

Comment 11

[Jim Mathies [:jimm]]

(In reply to comment #9)
> Any time we get a paint event that wasn't preceded by an nsIFrame::Invalidate
> call we will try to do an empty transaction. I bet Windows sends us a paint
> event when we become inactive?

We generate this in widget when we receive an activation state change. The nc areas we paint are invalidated.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix?

Does this fix it?

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think we should do something here even if it doesn't fix this bug. Maybe instead of all this leading down to FrameLayerBuilder::InvalidateAllThebesLayerContents, we should call root->InvalidateFrameSubtree() (which didn't exist when this code was written) and remove FrameLayerBuilder::InvalidateAllThebesLayerContents completely.

Comment 15

[Timothy Nikkel (:tnikkel)]

Yeah, that fixes it.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Well then, this should fix it too.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Remove unused API

Comment 18

[Timothy Nikkel (:tnikkel)]

Comment on attachment 509239 [details] [diff] [review]
Remove unused API

I think you might need to remove more code. I think there is one instance of InvalidateAllThebesLayerContents in a comment that this patch does not remove.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #18)
> Comment on attachment 509239 [details] [diff] [review]
> Remove unused API
> 
> I think you might need to remove more code. I think there is one instance of
> InvalidateAllThebesLayerContents in a comment that this patch does not remove.

Where?

Comment 21

[Timothy Nikkel (:tnikkel)]

http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp#746

I don't see that in your diff.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: updated patch

Sorry, I had updated my patch locally and forgot to upload it.

Comment 23

[Steve Scott (pxbugz)]

Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre

Comment 25

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Second patch landed:
http://hg.mozilla.org/mozilla-central/rev/3268bdbd64c4