1112556 - Ctrl-Tab previews are oversized with a small number of tabs open

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Dão Gottwald [:dao]]

Steps to reproduce:

- in about:config, set browser.ctrlTab.previews to true
- with with three tabs open, hold Ctrl and hit Tab to open the Ctrl-Tab panel
- open another tab and re-open the Ctrl-Tab panel repeatedly

-> observe the Ctrl-Tab previews getting smaller until you reach six tabs

I think it would make more sense to always have the previews at the same size (as if there were six tabs open).

All the relevant code is located here:
https://hg.mozilla.org/mozilla-central/file/tip/browser/base/content/browser-ctrlTab.js

To fix this bug, a maxTabPreviews getter should be added to the ctrlTab object and it should return this.previews.length - 1 (minus 1 because the last preview is not a tab preview but the "Show all X tabs" button). Then, maxTabPreviews should be used in tabPreviewCount instead of this.previews.length - 1 and in canvasWidth instead of tabPreviewCount; the last part is what will actually fix this bug.

Comment 1

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 1 - Ctrl-Tab previews are oversized

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8538118 [details] [diff] [review]
rev 1 - Ctrl-Tab previews are oversized

Since canvasWidth, canvasHeight and maxTabPreviews won't ever change, can you make them "smart" getters such that we won't needlessly do these calculations again and again? See for instance the "previews" getter.

Comment 3

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 2 - Ctrl-Tab previews are oversized

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 8538554 [details] [diff] [review]
rev 2 - Ctrl-Tab previews are oversized

>+  get maxTabPreviews () {
>+    return this.maxTabPreviews = this.previews.length - 1;
>+  },

>+  get canvasWidth () {
>+    return this.canvasWidth = Math.ceil(screen.availWidth * .85 / this.maxTabPreviews);
>+  },
>+  get canvasHeight () {
>+    return this.canvasHeight = Math.round(this.canvasWidth * tabPreviews.aspectRatio);
>+  },

You need to delete the getters (e.g. delete this.maxTabPreviews;) before setting the properties. Otherwise this won't have an effect.

Also, could you please move these three getters up to the other smart/lazy getters (e.g. after the "previews" getter)?

Comment 5

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 3 - Ctrl-Tab previews are oversized

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 8538615 [details] [diff] [review]
rev 3 - Ctrl-Tab previews are oversized

Looks perfect, thanks!

Comment 7

[Abdelrahman Ahmed [:abdelrahman]]

I need to be vouched for Mozillians community, can you help me for that?!
https://mozillians.org/en-US/u/codo.abdo/

Comment 8

[Dão Gottwald [:dao]]

(In reply to Abdelrhman from comment #7)
> I need to be vouched for Mozillians community, can you help me for that?!
> https://mozillians.org/en-US/u/codo.abdo/

Sure, done!

Comment 9

[Abdelrahman Ahmed [:abdelrahman]]

Thanks, for any upcoming bugs just tell me to work on.

Comment 10

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/08d1470a4042

(In reply to Abdelrhman from comment #9)
> Thanks, for any upcoming bugs just tell me to work on.

I'll keep an eye out and let you know if I find something.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Something in this push caused mass bustage. Backed out.
https://hg.mozilla.org/integration/fx-team/rev/e4c72ee3f315

Comment 12

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/a00723e1a186

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a00723e1a186

Comment 14

[Mohammad Maruf Islam]

I have reproduced this bug with Firefox Nightly 37.0a1 (Build ID: 20141217030202) on 
windows 8.1 64-bit with the instructions from comment 0 .

Verified as fixed with Firefox Nightly 45.0a1 (Build ID: 20151208030212)

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0

Comment 15

[Khalid Syfullah Zaman [:khalid32]]

I have reproduced this bug on Nightly 37.0a1 (2014-12-17) on ubuntu 14.04 LTS, 32 bit!

The bug's fix is now verified on Latest Nightly 46.0a1!

Build ID: 20151216030229
User Agent: Mozilla/5.0 (X11; Linux i686; rv:46.0) Gecko/20100101 Firefox/46.0

Marked this bug as verified!