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

VERIFIED FIXED

Status

()

defect
VERIFIED FIXED
9 years ago
6 years ago

People

(Reporter: davhorv112, Assigned: roc)

Tracking

({regression})

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

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [hardblocker][fx4-fixed-bugday])

Attachments

(2 attachments, 2 obsolete attachments)

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.
What windows theme are you using?
The default Luna theme.

I checked with Firefox beta 8 and using this there is no problem.
confirmed, this should block.
Assignee: nobody → jmathies
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
Summary: Switch between active and incative title bar doen't work → Switching main window between active and inactive doesn't update the title bar active state
blocking2.0: ? → final+
Whiteboard: [hardblocker]
Can anyone else confirm this goes away after the browser sits open for about a minute?
Ah, never mind. If you have the cursor focus in the search edit, the problem goes away. Clearly some sort of invalidation/repainting problem.
(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
Blocks: 615870
tn, can you take this?
Assignee: jmathies → 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?
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.
(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.
Duplicate of this bug: 626981
Posted patch fix? (obsolete) — Splinter Review
Does this fix it?
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.
Yeah, that fixes it.
Posted patch fixSplinter Review
Well then, this should fix it too.
Assignee: tnikkel → roc
Attachment #509072 - Attachment is obsolete: true
Attachment #509232 - Flags: review?(tnikkel)
Attachment #509232 - Flags: review?(tnikkel) → review+
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.
Keywords: checkin-needed
Whiteboard: [hardblocker] → [hardblocker][needs landing]
Whiteboard: [hardblocker][needs landing] → [hardblocker][has patch]
http://hg.mozilla.org/mozilla-central/rev/c5b8aa1d4c8a
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(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?
Posted patch updated patchSplinter Review
Sorry, I had updated my patch locally and forgot to upload it.
Attachment #509239 - Attachment is obsolete: true
Attachment #509514 - Flags: review?(tnikkel)
Attachment #509239 - Flags: review?(tnikkel)
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre
Status: RESOLVED → VERIFIED
Whiteboard: [hardblocker][has patch] → [hardblocker][has patch][fx4-fixed-bugday]
Duplicate of this bug: 631321
Attachment #509514 - Flags: review?(tnikkel) → review+
Whiteboard: [hardblocker][has patch][fx4-fixed-bugday] → [hardblocker][has patch][fx4-fixed-bugday][needs landing]
Keywords: checkin-needed
Whiteboard: [hardblocker][has patch][fx4-fixed-bugday][needs landing] → [hardblocker][fx4-fixed-bugday][second patch needs landing][needs landing]
Second patch landed:
http://hg.mozilla.org/mozilla-central/rev/3268bdbd64c4
Keywords: checkin-needed
Whiteboard: [hardblocker][fx4-fixed-bugday][second patch needs landing][needs landing] → [hardblocker][fx4-fixed-bugday]
You need to log in before you can comment on or make changes to this bug.