Closed
Bug 631932
Opened 14 years ago
Closed 14 years ago
Aero Peek tab previews are always enabled after entering and exiting Panorama
Categories
(Firefox Graveyard :: Panorama, defect)
Tracking
(blocking2.0 final+)
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: rain1, Assigned: jimm)
References
Details
(Whiteboard: [hardblocker])
Attachments
(1 file)
(deleted),
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
STR:
1. Disable Aero Peek tab previews.
2. Enter Panorama.
3. Exit Panorama.
The tab previews always remain enabled.
Pretty sure the bug is at https://hg.mozilla.org/mozilla-central/rev/953be21166fe#l1.57 -- you shouldn't be changing enabled and should probably instead have a separate _tempEnabled bool.
Reporter | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Assignee | ||
Comment 1•14 years ago
|
||
Oops, I'll get a fix in this week. Thanks Sid. Drivers, this should block final.
Assignee: nobody → jmathies
Updated•14 years ago
|
blocking2.0: ? → final+
Whiteboard: [hardblocker]
Comment 3•14 years ago
|
||
Comment on attachment 510263 [details] [diff] [review]
patch
What if checkPreviewCount is called while in panorama? What if panorama is shown/hidden in two windows?
Assignee | ||
Comment 4•14 years ago
|
||
(In reply to comment #3)
> Comment on attachment 510263 [details] [diff] [review]
> patch
>
> What if checkPreviewCount is called while in panorama? What if panorama is
> shown/hidden in two windows?
I didn't see any issues testing this, with the pref off, tab previews stay off, with it on, we get per window tab previews disabled when panorama is enabled. Enabling and disabling panorama in multiple windows doesn't break anything.
checkPreviewCount: function () {
if (this.previews.length > this.maxpreviews)
this.enabled = false;
else
this.enabled = this._prefenabled;
},
If _prefenabled is false, enabled will always be false.
Comment 5•14 years ago
|
||
(In reply to comment #4)
> If _prefenabled is false, enabled will always be false.
And if _prefenabled is true?
Assignee | ||
Comment 6•14 years ago
|
||
(In reply to comment #5)
> (In reply to comment #4)
> > If _prefenabled is false, enabled will always be false.
>
> And if _prefenabled is true?
Doesn't appear to be an issue. That sets AeroPeeks's this.enabled to this._prefenabled, which calls the enabled setter. The setter then does this:
if (this._enabled == enable)
return;
So nothing changes. Is there some specific STR case you see you would like me to test?
Comment 7•14 years ago
|
||
Comment on attachment 510263 [details] [diff] [review]
patch
I was confusing TabWindow.prototype.enabled with AeroPeek.enabled.
Attachment #510263 -
Flags: review?(dao) → review+
Updated•14 years ago
|
Whiteboard: [hardblocker] → [hardblocker][has patch]
Assignee | ||
Comment 8•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [hardblocker][has patch] → [hardblocker]
Comment 11•14 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110215 Firefox/4.0b12pre
Verified issue and it's no longer reproducible.
Status: RESOLVED → VERIFIED
Updated•9 years ago
|
Product: Firefox → Firefox Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•