1256714 - Aero Peek not working

Status: RESOLVED FIXED

(SeaMonkey :: OS Integration, defect)

Description

[Frank-Rainer Grahl (:frg)]

Attachment: 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.

Comment 1

[Rainer Bielefeld]

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

Comment 2

[Frank-Rainer Grahl (:frg)]

Thanks for confirmation. I think we can safely set it to new then. I doubt any official builds will behave otherwise.

Comment 3

[Philip Chee]

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

Comment 4

[Frank-Rainer Grahl (:frg)]

Hmm I tried this with a modified WindowsPreviewPerTab.jsm from FF after filing the bug and didn't succeed. Will look into this.

Comment 5

[Frank-Rainer Grahl (:frg)]

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?

Comment 6

[Frank-Rainer Grahl (:frg)]

Attachment: 1256714-Aero-Peek.patch

Patch which works for me and uses the unmodified Firefox WindowsPreviewPerTab.jsm.

Comment 7

[Philip Chee]

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

Comment 8

[Frank-Rainer Grahl (:frg)]

>> 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

Comment 9

[Frank-Rainer Grahl (:frg)]

>> 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.

Comment 10

[Frank-Rainer Grahl (:frg)]

Attachment: WIP-20160504.patch

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.

Comment 11

[Frank-Rainer Grahl (:frg)]

Attachment: 1256714-AeroPeek.patch

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).

Comment 12

[Frank-Rainer Grahl (:frg)]

Attachment: 1256714-AeroPeek-V2.patch

Updated patch because of recent security fix in Firefox. See Bug 1283067.

Comment 13

[Frank-Rainer Grahl (:frg)]

Attachment: 1256714-AeroPeek-V3.patch

Bug 1285196 included

Comment 14

[Ian Neal]

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

Comment 15

[Frank-Rainer Grahl (:frg)]

Attachment: 1256714-AeroPeek-V4.patch

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?

Comment 16

[Ian Neal]

Comment on attachment 8774155 [details] [diff] [review]
1256714-AeroPeek-V4.patch

[Triage Comment]
r/a=me for c-c, c-a and c-b

Comment 17

[Ian Neal]

Comment on attachment 8774155 [details] [diff] [review]
1256714-AeroPeek-V4.patch

[Triage Comment]
a=me for c-r too

Comment 18

[Frank-Rainer Grahl (:frg)]

Fixed for c-c. Stay tuned for other trees.

https://hg.mozilla.org/comm-central/rev/c93c2e0021a6

Comment 19

[Frank-Rainer Grahl (:frg)]

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.

Comment 20

[Rainer Bielefeld]

TM for easing queries.

Comment 21

[Rainer Bielefeld]

Please see Bug 1293618 comment #38