Set the default of `browser.tabs.tabMinWidth` to 76.

RESOLVED FIXED in Firefox 57

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mconley, Assigned: dao)

Tracking

(Depends on 1 bug)

unspecified
Firefox 58
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox57 fixed, firefox58 fixed)

Details

Attachments

(3 attachments, 2 obsolete attachments)

+++ This bug was initially created as a clone of Bug #1404465 +++

If you want to debate the default setting, please don't do that in this bug or bug 1404465. Please do that in this dev-platform thread instead: https://groups.google.com/forum/#!topic/mozilla.dev.platform/4RAQOAMT9PI
Request to do this was made by canuckistani and signed off by shorlander - see bug 1404465 comment 47.
Assignee

Comment 2

2 years ago
(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from comment #1)
> Request to do this was made by canuckistani and signed off by shorlander -
> see bug 1404465 comment 47.

It's not gonna fly, see bug 1404465 comment 59.
Given comment 2, how should I proceed?
Flags: needinfo?(shorlander)
Flags: needinfo?(jgriffiths)

Comment 4

2 years ago
While 70 allows me to see more tabs at once, it's not necessarily helpful when you have multiple similar tabs open. I often open up multiple gmail tabs with different accounts, and what I end up seeing is "Inbox", "Inbox", "Inbox"... which is not very helpful. I increased the width to 90, and I could start to see the inbox count, which was more helpful.

I guess this is an use case that could be covered by container tabs, but unfortunately, container tabs don't have great integration with bookmarks and the awesomebar (would be nice to have suggestions like: Gmail in Work container"), and automatically opening URIs in containers isn't helpful either, because Gmail has the same URI across containers.

Another consequence of decreasing the max tab width for me was that I had more duplicate tabs opened, and discouraged me to clean up my tabs periodically.


A min-width of 90 (lower than the previous 100px), works well for my purpose.
Thanks ntim.

Note that I _really_ encourage this sort of conversation to happen in the dev-platform thread at https://groups.google.com/forum/#!topic/mozilla.dev.platform/4RAQOAMT9PI rather than here. I don't really want this bug to spiral out into discussion and debate like bug 1404465.
Haven't heard back from shorlander or canuckistani here, but I've posted the patch for review anyway. Pretty easy to change the number if they come back with something different, I guess.

I'm away on Monday due to a holiday, but anyone should feel free to land (or steal, correct and land) if the desired number comes in.
Posted patch 1406478-for-beta.diff (obsolete) — Splinter Review
Here's this patch _and_ the patch for bug 1404465 folded and re-based for mozilla-beta, in the event that this needs to be landed on Monday and I'm not around due to the holiday.

Again, please feel free to steal, modify, request uplift, and land in the event that different numbers come down.
Assignee

Comment 9

2 years ago
mozreview-review
Comment on attachment 8916180 [details]
Bug 1406478 - Set the browser.tabs.tabMinWidth default to 70 (but allow it to go as low as 50).

https://reviewboard.mozilla.org/r/187432/#review192524

::: browser/app/profile/firefox.js:491
(Diff revision 1)
>  pref("browser.tabs.showAudioPlayingIcon", true);
>  // This should match Chromium's audio indicator delay.
>  pref("browser.tabs.delayHidingAudioPlayingIconMS", 3000);
>  
>  // The minimum tab width in pixels
> -pref("browser.tabs.tabMinWidth", 50);
> +pref("browser.tabs.tabMinWidth", 70);

In touch mode, a selected tab with a favicon and the sound playing icon seems to consume 82 pixels in my testing on Ubuntu (but this should be consistent across platforms). That would be the lowest value we can set without other modifications to the tab layout.
Attachment #8916180 - Flags: review-
(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from comment #8)
> Created attachment 8916190 [details] [diff] [review]
> 1406478-for-beta.diff
> 
> Here's this patch _and_ the patch for bug 1404465 folded and re-based for
> mozilla-beta, in the event that this needs to be landed on Monday and I'm
> not around due to the holiday.
> 
> Again, please feel free to steal, modify, request uplift, and land in the
> event that different numbers come down.

We're good to go for a default setting of 75, I've adjusted the description to reflect this.
Flags: needinfo?(jgriffiths)
Mike: this can wait until Tuesday but no later. I'll circle back around with you on Tuesday morning.
Flags: needinfo?(mconley)
Summary: Set the default of `browser.tabs.tabMinWidth` to 70. → Set the default of `browser.tabs.tabMinWidth` to 75.
Aren't comment 9 and comment 10 contradictory?

> 82 pixels [...] would be the lowest value we can set

> We're good to go for a default setting of 75
Assignee

Comment 13

2 years ago
(In reply to Oriol Brufau [:Oriol] from comment #12)
> Aren't comment 9 and comment 10 contradictory?
> 
> > 82 pixels [...] would be the lowest value we can set
> 
> > We're good to go for a default setting of 75

75 works with compact and normal UI density (I think) but for touch we'll need more space.
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Flags: needinfo?(shorlander)
Flags: needinfo?(mconley)
Assignee

Updated

2 years ago
Attachment #8916180 - Attachment is obsolete: true
Attachment #8916180 - Flags: review?(jaws)
Assignee

Updated

2 years ago
Attachment #8916190 - Attachment is obsolete: true
Assignee

Comment 14

2 years ago
(In reply to Dão Gottwald [::dao] from comment #13)
> (In reply to Oriol Brufau [:Oriol] from comment #12)
> > Aren't comment 9 and comment 10 contradictory?
> > 
> > > 82 pixels [...] would be the lowest value we can set
> > 
> > > We're good to go for a default setting of 75
> 
> 75 works with compact and normal UI density (I think)

Hm, no, it doesn't. Bug 1391539 made the separators consume another pixel, so now 76 seems to be the minimum...
Comment hidden (mozreview-request)

Comment 17

2 years ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a4fc441d0663
Set browser.tabs.tabMinWidth to 76. r=jaws
Backed out for failing browser-chrome's browser/base/content/test/general/browser_tabReorder.js:

https://hg.mozilla.org/integration/autoland/rev/ea295817d104eda10909e6155369019cb7e0887a

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a4fc441d0663d647e0f734420bc1c1eff7951434&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=135725193&repo=autoland

[task 2017-10-09T16:36:32.057Z] 16:36:32     INFO - TEST-START | browser/base/content/test/general/browser_tabReorder.js
[task 2017-10-09T16:36:37.974Z] 16:36:37     INFO - TEST-INFO | started process screentopng
[task 2017-10-09T16:36:39.481Z] 16:36:39     INFO - TEST-INFO | screentopng: exit 0
[task 2017-10-09T16:36:39.485Z] 16:36:39     INFO - Buffered messages logged at 16:36:32
[task 2017-10-09T16:36:39.485Z] 16:36:39     INFO - Entering test bound 
[task 2017-10-09T16:36:39.485Z] 16:36:39     INFO - TEST-PASS | browser/base/content/test/general/browser_tabReorder.js | new tabs are opened - 
[task 2017-10-09T16:36:39.488Z] 16:36:39     INFO - TEST-PASS | browser/base/content/test/general/browser_tabReorder.js | newTab1 position is correct - 
[task 2017-10-09T16:36:39.490Z] 16:36:39     INFO - TEST-PASS | browser/base/content/test/general/browser_tabReorder.js | newTab2 position is correct - 
[task 2017-10-09T16:36:39.498Z] 16:36:39     INFO - TEST-PASS | browser/base/content/test/general/browser_tabReorder.js | newTab3 position is correct - 
[task 2017-10-09T16:36:39.500Z] 16:36:39     INFO - Buffered messages finished
[task 2017-10-09T16:36:39.503Z] 16:36:39     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_tabReorder.js | Uncaught exception - Waiting for tab position to be updated - timed out after 50 tries.
Flags: needinfo?(dao+bmo)
Comment hidden (mozreview-request)

Comment 20

2 years ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d91a88f5f5d
Set browser.tabs.tabMinWidth to 76. r=jaws
Assignee

Updated

2 years ago
Flags: needinfo?(dao+bmo)
https://hg.mozilla.org/mozilla-central/rev/2d91a88f5f5d
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Title of bug is misleading in that the final size was set to 76, not 75
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #22)
> Title of bug is misleading in that the final size was set to 76, not 75

Changed it to reflect commit's change.
Summary: Set the default of `browser.tabs.tabMinWidth` to 75. → Set the default of `browser.tabs.tabMinWidth` to 76.
Assignee

Comment 24

2 years ago

Updated

2 years ago
Depends on: 1406591
Assignee

Updated

2 years ago
No longer depends on: 1406591
It looks like this most recent patch still makes the title invisible in touch mode with a favicon and tab mute icon... what was the plan there?
Flags: needinfo?(dao+bmo)
Assignee

Comment 26

2 years ago
(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from comment #25)
> It looks like this most recent patch still makes the title invisible in
> touch mode with a favicon and tab mute icon... what was the plan there?

There's no plan that I know of :(
I just made it so that the default tab minimum width doesn't undercut the tab contents' minimum width. Is this not working on your end?
Flags: needinfo?(dao+bmo) → needinfo?(mconley)
Stephen, this is what it looks like as per Mike's comment. Is this acceptable?
Flags: needinfo?(shorlander)
Assignee

Comment 28

2 years ago
(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from comment #25)
> It looks like this most recent patch still makes the title invisible in
> touch mode with a favicon and tab mute icon... what was the plan there?

(In reply to Jeff Griffiths (:canuckistani) (:⚡︎) from comment #27)
> Created attachment 8917026 [details]
> Screen Shot 2017-10-10 at 11.58.08 AM.png
> 
> Stephen, this is what it looks like as per Mike's comment. Is this
> acceptable?

Note that this isn't limited to touch mode.
Comment on attachment 8916905 [details] [diff] [review]
patch for uplift

Approval Request Comment
[Feature/Bug causing the regression]:

None. This was something requested by mayo / canuckistani for 57.

[User impact if declined]:

Users are more likely to enter the "overflow" state for the browser tab strip, which is something we want to avoid when possible.

[Is this code covered by automated tests?]:

Yes.

[Has the fix been verified in Nightly?]:

Yes, I have manually verified this change.

[Needs manual test from QE? If yes, steps to reproduce]: 

No.

[List of other uplifts needed for the feature/fix]:

None.

[Is the change risky?]:

As far as I can tell, this is only risky for one reason (see below).

[Why is the change risky/not risky?]:

The only risk that we've identified is that in Touch Mode (browser.uidensity = 2), at least on my MBP, tabs that have both a favicon and the media mute icons (so, for example, a YouTube video playing music) hide the tab title. I'm waiting for confirmation on whether or not this is acceptable.

[String changes made/needed]:

None.
Attachment #8916905 - Flags: approval-mozilla-beta?
(In reply to Dão Gottwald [::dao] from comment #28)
> (In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from
> comment #25)
> > It looks like this most recent patch still makes the title invisible in
> > touch mode with a favicon and tab mute icon... what was the plan there?
> 
> (In reply to Jeff Griffiths (:canuckistani) (:⚡︎) from comment #27)
> > Created attachment 8917026 [details]
> > Screen Shot 2017-10-10 at 11.58.08 AM.png
> > 
> > Stephen, this is what it looks like as per Mike's comment. Is this
> > acceptable?
> 
> Note that this isn't limited to touch mode.

Yes, you're right - this seems to affect normal and compact mode as well.

So all modes.
Flags: needinfo?(mconley)
canuckistani says we're still a go for 76px min tab width for 57.
Comment on attachment 8916905 [details] [diff] [review]
patch for uplift

must fix, beta57+
Attachment #8916905 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee

Updated

2 years ago
No longer depends on: 1405923
Assignee

Updated

2 years ago
Duplicate of this bug: 1405923
(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from comment #29)
> [Is this code covered by automated tests?]:
> 
> Yes.
> 
> [Has the fix been verified in Nightly?]:
> 
> Yes, I have manually verified this change.
> 
> [Needs manual test from QE? If yes, steps to reproduce]: 
> 
> No.

Setting qe-verify- based on Mike's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-

Comment 36

2 years ago
I have reproduced this bug with Nightly 58.0a1 (2017-10-06) on Windows 10 , 64 Bit !

This bug's fix is Verified with latest Beta & Nightly !

Build   ID    20171023180840
User Agent    Mozilla/5.0 (Windows NT 10.0; WOW64; rv:57.0) Gecko/20100101 Firefox/57.0

Build   ID    20171025100449
User Agent    Mozilla/5.0 (Windows NT 10.0; WOW64; rv:58.0) Gecko/20100101 Firefox/58.0

[bugday-20171025]
Assignee

Updated

2 years ago
Depends on: 1415671
You need to log in before you can comment on or make changes to this bug.