Closed Bug 1256714 Opened 4 years ago Closed 3 years ago

Aero Peek not working

Categories

(SeaMonkey :: OS Integration, defect)

SeaMonkey 2.43 Branch
Unspecified
Windows
defect
Not set

Tracking

(seamonkey2.44 wontfix, seamonkey2.45 fixed, seamonkey2.46 fixed, seamonkey2.47 fixed, seamonkey2.48 fixed)

RESOLVED FIXED
seamonkey2.48
Tracking Status
seamonkey2.44 --- wontfix
seamonkey2.45 --- fixed
seamonkey2.46 --- fixed
seamonkey2.47 --- fixed
seamonkey2.48 --- fixed

People

(Reporter: frg, Assigned: frg)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 5 obsolete files)

Attached image Clipboard.jpg
Starting with 2.43 Aero peek is not fully working for me. The Taskbar Thumbnail Live previews show only a swirling icon and then the site icon after some time which means something did go wrong. Chatzilla will show the normal preview and alt-tab also shows the preview windows. Just the taskbar is broken.

Happens under Windows 7 and 10 using VS2013 and VS2015 x86 and x64 builds.

2.42 is fine.

Unconfirmed for now because not using an official build.
REPRODUCIBLE with unofficial (from <http://seamonkey.callek.net/contrib/>)  en-US SeaMonkey 2.44a1  Mozilla/5.0 (Windows NT 6.1; x64; rv:47.0)  Gecko/20100101 Firefox/47.0 Build 20160208090255  (Default Classic Theme)  on German WIN7 64bit

REPRODUCIBLE with unofficial (from <http://seamonkey.callek.net/contrib/>)  en-US SeaMonkey 2.44a1  Mozilla/5.0 (Windows NT 6.1; WOW64; rv:47.0)  Gecko/20100101 Firefox/47.0 Build 20160205232941  (Default Classic Theme)  on German WIN7 64bit
Thanks for confirmation. I think we can safely set it to new then. I doubt any official builds will behave otherwise.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Bug 1096804 - [e10s] Support windows taskbar previews and thumbnails

We need to port the changes to our version of WindowsPreviewPerTab.jsm:
https://hg.mozilla.org/mozilla-central/rev/f077e048a451
https://hg.mozilla.org/mozilla-central/rev/49de4e1f40e6
Depends on: 1096804
Hmm I tried this with a modified WindowsPreviewPerTab.jsm from FF after filing the bug and didn't succeed. Will look into this.
Hmm2, the unmodified WindowsPreviewPerTab.jsm from FF works for suite. I didn't see anything suite or FF specific in it too. Just use it 1:1 as a quick fix or do some porting?
Flags: needinfo?(philip.chee)
Attached patch 1256714-Aero-Peek.patch (obsolete) — Splinter Review
Patch which works for me and uses the unmodified Firefox WindowsPreviewPerTab.jsm.
Neil would not agree to just pasting on the unmodified Firefox WindowsPreviewPerTab.jsm.
You would need to port Bug 1096804
https://hg.mozilla.org/mozilla-central/rev/f077e048a451
Flags: needinfo?(philip.chee)
>> Neil would not agree to just pasting on the unmodified Firefox WindowsPreviewPerTab.jsm.

I think they were the same once and will end up very similar after porting. I will look into it. The current code still has a problem with sites where you did set a custom zoom level. Painting the main window fails if you hover over the preview thumb. I will see that I find/fix this bug first and then remodel the old jsm if no one else has taken the bug in the meantime.

FRG
Depends on: 1269810
>> The current code still has a problem with sites where you did set a custom zoom level.

This is a bug in the backend. Also occurs with FF 46. Filed bug 1269810 for Firefox 46.
Attached patch WIP-20160504.patch (obsolete) — Splinter Review
I copied the latest FF WindowsPreviewPerTab.jsm and adapted it to Seamonkey. Should I go forward with this one or do additional adaptions in the last part which I didn't change like:

>> +XPCOMUtils.defineLazyGetter(AeroPeek, "cacheTimer", () => ..
>> ...
>> +XPCOMUtils.defineLazyServiceGetter(AeroPeek, "prefs",

This is still not 100% working but the remaining problem with zoom seems to be a core problem as stated in comment 9. I can't fine a fault here but if yes another bug could take care of it.
Attachment #8738182 - Attachment is obsolete: true
Attachment #8748866 - Flags: feedback?(philip.chee)
Attached patch 1256714-AeroPeek.patch (obsolete) — Splinter Review
Well I doubt there will be any other takers soon so lets just make this an official patch.

Problems with zoom are reported in Bug 1269810 against Firefox (likely a toolkit bug).
Assignee: nobody → frgrahl
Attachment #8748866 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8748866 - Flags: feedback?(philip.chee)
Attachment #8763366 - Flags: review?(philip.chee)
Attached patch 1256714-AeroPeek-V2.patch (obsolete) — Splinter Review
Updated patch because of recent security fix in Firefox. See Bug 1283067.
Attachment #8763366 - Attachment is obsolete: true
Attachment #8763366 - Flags: review?(philip.chee)
Attachment #8769419 - Flags: review?(philip.chee)
Attached patch 1256714-AeroPeek-V3.patch (obsolete) — Splinter Review
Bug 1285196 included
Attachment #8769419 - Attachment is obsolete: true
Attachment #8769419 - Flags: review?(philip.chee)
Attachment #8771567 - Flags: review?(philip.chee)
Attachment #8771567 - Flags: feedback?(iann_bugzilla)
Comment on attachment 8771567 [details] [diff] [review]
1256714-AeroPeek-V3.patch

>+++ b/suite/browser/tabbrowser.xml

>+          <![CDATA[
>+            let currentTab = this.selectedTab;
>+            try {
>+              // Suppress focus, ownership and selected tab changes
Nit: missing full stop at the end of the comment.

>+++ b/suite/modules/WindowsPreviewPerTab.jsm

>+  } catch (e) {
>+    // Ignore channels which do not support nsIPrivateBrowsingChannel
Nit: missing full stop at the end of the comment.

>+  // window width and height, not browser
Nit: comment should be just about width.
>   get width() {
>     return this.win.width;
>   },
> 
>+  // window width and height, not browser
Nit: comment should be just about height.
>   get height() {
>     return this.win.height;
>   },

>+      // Deliver the resulting composite canvas to Windows
Nit: missing full stop at the end of the comment.

>   //// nsIDOMEventListener
>   handleEvent: function (evt) {
>     switch (evt.type) {
>+      case "TabAttrModified":
>         this.updateTitleAndTooltip();
>         break;
Is it worth having a one case switch?


>   resetCacheTimer: function () {

>-    this.cacheTimer.init(this, 1000 * this.cacheLifespan,
>+    this.cacheTimer.cancel();
>+    this.cacheTimer.init(this, 1000*this.cacheLifespan,
Nit: keep spaces around the *
>                          Components.interfaces.nsITimer.TYPE_ONE_SHOT);
f=me so far
Attachment #8771567 - Flags: feedback?(iann_bugzilla) → feedback+
Fixed the nits and a few more but left the switch in. Would prefer if the actual codes stays as close as possible to the Firefox one until we need some different functionality. Makes backporting fixes really easy.

>>   //// nsIDOMEventListener
>>   handleEvent: function (evt) {
>>     switch (evt.type) {
>>+      case "TabAttrModified":
>>         this.updateTitleAndTooltip();
>>         break;
> Is it worth having a one case switch?
Attachment #8771567 - Attachment is obsolete: true
Attachment #8771567 - Flags: review?(philip.chee)
Attachment #8774155 - Flags: review?(iann_bugzilla)
Attachment #8774155 - Flags: review?(philip.chee)
Comment on attachment 8774155 [details] [diff] [review]
1256714-AeroPeek-V4.patch

[Triage Comment]
r/a=me for c-c, c-a and c-b
Attachment #8774155 - Flags: review?(iann_bugzilla)
Attachment #8774155 - Flags: review+
Attachment #8774155 - Flags: approval-comm-beta+
Attachment #8774155 - Flags: approval-comm-aurora+
Comment on attachment 8774155 [details] [diff] [review]
1256714-AeroPeek-V4.patch

[Triage Comment]
a=me for c-r too
Attachment #8774155 - Flags: approval-comm-release+
Fixed for c-c. Stay tuned for other trees.

https://hg.mozilla.org/comm-central/rev/c93c2e0021a6
https://hg.mozilla.org/releases/comm-aurora/rev/7744817332d7
https://hg.mozilla.org/releases/comm-beta/rev/a170473798d0
https://hg.mozilla.org/releases/comm-release/rev/e8030ba9c303

This part is fixed. I hope the Mozilla devs will see that the zoom problem also gets fixed. If it turns out not to be a toolkit bug I will open a followup bug.
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Attachment #8774155 - Flags: review?(philip.chee)
TM for easing queries.
Target Milestone: --- → seamonkey2.45
Target Milestone: seamonkey2.45 → seamonkey2.48
Please see Bug 1293618 comment #38
Target Milestone: seamonkey2.48 → seamonkey2.45
Target Milestone: seamonkey2.45 → seamonkey2.48
You need to log in before you can comment on or make changes to this bug.