Remove drag space above the tabs

RESOLVED FIXED in Thunderbird 56.0

Status

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

unspecified
Thunderbird 56.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Like FX has now, we could also remove the drag space above the tabs.
Posted patch dragSpace.patch (obsolete) — Splinter Review
Jörg, what do you think about this. Naturally you see only a difference when drawintitlebar is enabled.

Differently to FX I moved the drag space to the end of the tabs instead the start because TB has normally not so many tabs open and this space at the start looks not so good.

I also synchronized the space calculation code with the one from FX (browser-tabsintitlebar.js). One question, is it possible to get the "CAN_DRAW_IN_TITLEBAR" through appconstants? Then we could get rid of the precompiling of this file.

This works only on macOS and Windows because Linux can't draw into the titlebar
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8891987 - Flags: review?(jorgk)
(In reply to Richard Marti (:Paenglab) from comment #1)
> Jörg, what do you think about this. Naturally you see only a difference when
> drawintitlebar is enabled.
To see the difference, I need to switch the menu off, right?
(Note, I generally use a menu and mail.tabs.drawInTitlebar==false, so please mention differing settings in your instructions).

Where did FF introduce this, it's not in version 54. I think it looks better, since we save space and the minimise, maximise and close icons move down into the space provided for them in the tabbar.
 
> Differently to FX I moved the drag space to the end of the tabs instead the
> start because TB has normally not so many tabs open and this space at the
> start looks not so good.
This I don't understand, what's "drag space"? I don't have a current FF Nightly installed, perhaps you can attach a screen shot.

> I also synchronized the space calculation code with the one from FX
> (browser-tabsintitlebar.js). One question, is it possible to get the
> "CAN_DRAW_IN_TITLEBAR" through appconstants? Then we could get rid of the
> precompiling of this file.
Hmm, I can see |if (AppConstants.CAN_DRAW_IN_TITLEBAR) {| in CustomizeMode.jsm. Does that not work in TB? If not, maybe consult Aceman.
Posted image Drag space on FX
(In reply to Jorg K (GMT+2) from comment #2)
> (In reply to Richard Marti (:Paenglab) from comment #1)
> Where did FF introduce this, it's not in version 54. I think it looks
> better, since we save space and the minimise, maximise and close icons move
> down into the space provided for them in the tabbar.

It's in the Nightlies behind the Photon build switch since 55.

> > Differently to FX I moved the drag space to the end of the tabs instead the
> > start because TB has normally not so many tabs open and this space at the
> > start looks not so good.
> This I don't understand, what's "drag space"? I don't have a current FF
> Nightly installed, perhaps you can attach a screen shot.

Attached.

> > I also synchronized the space calculation code with the one from FX
> > (browser-tabsintitlebar.js). One question, is it possible to get the
> > "CAN_DRAW_IN_TITLEBAR" through appconstants? Then we could get rid of the
> > precompiling of this file.
> Hmm, I can see |if (AppConstants.CAN_DRAW_IN_TITLEBAR) {| in
> CustomizeMode.jsm. Does that not work in TB? If not, maybe consult Aceman.

Ah great, then I try a new patch.
> Attached.
Make it smaller so can see better what's happening :-(
But I get the idea. There is some funny space to the left of the left-most tab, right? Ugly, no?
(In reply to Jorg K (GMT+2) from comment #4)
> > Attached.
> Make it smaller so can see better what's happening :-(
> But I get the idea. There is some funny space to the left of the left-most
> tab, right? Ugly, no?

Yes, that's why I moved it to the right of the tabs. It's only remarkable when there are enough tabs to fill the tabbar.
The same patch with AppConstants instead of #ifdef.
Attachment #8891987 - Attachment is obsolete: true
Attachment #8891987 - Flags: review?(jorgk)
Attachment #8892041 - Flags: review?(jorgk)
Comment on attachment 8892041 [details] [diff] [review]
dragSpace.patch

Thanks this works for me on Windows 10. Thanks for the patch without the white-space changes.

I have checked that you've synchronised code with M-C code from
mozilla-central/browser/base/content/browser-tabsintitlebar.js
Attachment #8892041 - Flags: review?(jorgk) → review+
If you want we can wait and land it in TB 57.
Keywords: checkin-needed
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/045fa1d535b5
Remove drag space above the tabs. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(In reply to Richard Marti (:Paenglab) from comment #9)
> If you want we can wait and land it in TB 57.
Why wait?

I'll even add this to the release notes for TB 56 this time:
Various improvements in the Thunderbird themes on all platforms, especially for Windows 10.
Target Milestone: --- → Thunderbird 56.0
Depends on: 1389883
You need to log in before you can comment on or make changes to this bug.