Tabbed mail in 3-pane window does no longer observe browser.tabs.autoHide for display of single tabs

VERIFIED FIXED in seamonkey2.24

Status

VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: rsx11m.pub, Assigned: neil)

Tracking

({regression})

SeaMonkey 2.22 Branch
seamonkey2.24
regression

SeaMonkey Tracking Flags

(seamonkey2.21 unaffected, seamonkey2.22 verified, seamonkey2.23 verified, seamonkey2.24 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

812 bytes, patch
mnyromyr
: review+
philip.chee
: feedback+
iann_bugzilla
: approval-comm-aurora+
iann_bugzilla
: approval-comm-beta+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
Tabbed mail used to observe the browser.tabs.autoHide preference to determine whether or not to show the tabbar when only a single tab is active.

Noticed on aurora nightly builds after rotating versions with the last merge:

 - 2.21a2 still shows no tabbar for a single tab with browser.tabs.autoHide set
 - 2.22a2 shows the tabbar for a single tab despite browser.tabs.autoHide set

Earliest build of that branch I have exhibiting the problem: 2.22a1 20130712
(Reporter)

Updated

5 years ago
status-seamonkey2.21: --- → unaffected
status-seamonkey2.22: --- → affected
status-seamonkey2.23: --- → affected
This bug also seems to be present in Firefox 23.0 (I can raise a separate bug if you prefer).
(Reporter)

Comment 2

5 years ago
Firefox doesn't have a Mail & News component (SeaMonkey's browser isn't affected). What you are most likely looking at is the effect of bug 855370 - Remove the ability to not "Always show the tab bar" - which is separate from SeaMonkey's implementation.

Also, given that SeaMonkey 2.20 isn't affected, this doesn't appear to be a fallout from the Firefox bug, unless they have removed supporting Core parts in the process (but then again, I'd expect the browser tabbar to behave the same way).

Thus, while Firefox removed the functionality on purpose, it should be a genuine regression in SeaMonkey's Mail & News component.

BTW: https://addons.mozilla.org/firefox/addon/hide-tab-bar-with-one-tab/ adds the lost functionality back to Firefox, but that's yet another add-on needed there.
(Reporter)

Comment 3

5 years ago
Works:  Linux i686 trunk nightly build of 2013-06-29;
Broken: Linux i686 trunk nightly build of 2013-07-01.

There was no 2013-06-30 trunk nightly build for any platform.

http://hg.mozilla.org/comm-central/pushloghtml?startdate=2013-06-29&enddate=2013-07-01+05%3A00
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2013-06-29&enddate=2013-07-01+05%3A00
Keywords: regressionwindow-wanted

Comment 4

5 years ago
fallout from Bug 653881 (Fix up our XBL1 shadow tree implementation so that we can reuse it for XBL2 )

http://hg.mozilla.org/comm-central/annotate/d2fe0ebc74db/suite/mailnews/tabmail.xml#l191

          <xul:tabs class="tabbrowser-tabs tabmail-tabs"
......
            <xul:tab selected="true"
......
            <children/>

After bug 653881, <children> is an explicit DOM node. With one tab open there are now two childnodes.

http://hg.mozilla.org/comm-central/annotate/d2fe0ebc74db/suite/mailnews/tabmail.xml#l273

      <property name="mAutoHide" onget="return this._mAutoHide;">
        <setter>
          <![CDATA[
            if (val != this._mAutoHide)
            {
              if (this.tabContainer.childNodes.length == 1)
                this.mStripVisible = !val;
              this._mAutoHide = val;
            }
            return val;
          ]]>
        </setter>
      </property>

Changing this to:
              let tabs = this.tabContainer.querySelectorAll(".tabmail-tab");
              if (tabs.length == 1)
Works but there are other places that need to be fixed

Updated

5 years ago
Blocks: 653881
(Assignee)

Comment 5

5 years ago
Created attachment 787836 [details] [diff] [review]
Possible patch

By inserting the tabs before the xbl:children node we at least get all the nodes consecutively.
Attachment #787836 - Flags: feedback?(philip.chee)

Comment 6

5 years ago
Comment on attachment 787836 [details] [diff] [review]
Possible patch

This works until I open a calender tab or two.

What about: <method name="_updateTabsVisibilityStatus"> ?

>       <handler event="popupshowing">
>         <![CDATA[
>           // set up the menu popup
>           let tabcontainer = document.getBindingParent(this);
>           let tabs = tabcontainer.childNodes;

The all tabs popdown menu has an extra blank entry at the end.
Attachment #787836 - Flags: feedback?(philip.chee) → feedback-
(Assignee)

Comment 7

5 years ago
Created attachment 797766 [details] [diff] [review]
Proposed patch

I had another look into this and the problem is worse than I thought; even the base <tabs> binding gets confused by the extra xbl:children child node. So the only solution I could find was to put the Calendar buttons after the dropdown. Thunderbird's tabs on top had to remove that trick with the rest of their xbl, which means that we're not missing out on anything.
Attachment #787836 - Attachment is obsolete: true
Attachment #797766 - Flags: feedback?(philip.chee)

Comment 8

5 years ago
Comment on attachment 797766 [details] [diff] [review]
Proposed patch

The UI is sub-optimal but at least it works.

Calendar tabs can be closed with the [x] close button or from the tab context menu. CTRL-W doesn't work on calendar tabs. But I think this problem is not related to this bug.
Attachment #797766 - Flags: feedback?(philip.chee) → feedback+
(Assignee)

Updated

5 years ago
Attachment #797766 - Flags: review?(mnyromyr)

Updated

5 years ago
Attachment #797766 - Flags: review?(mnyromyr) → review+
(Assignee)

Comment 9

5 years ago
Pushed comm-central changeset 781c3bd778c5.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-seamonkey2.24: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.24
(Assignee)

Comment 10

5 years ago
Comment on attachment 797766 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
Regression caused by (bug #): 653881
User impact if declined: Mail tabs don't operate correctly
Testing completed (on m-c, etc.): Port of Thunderbird change
Risk to taking this patch (and alternatives if risky): Low
String changes made by this patch: None
status-seamonkey2.24: fixed → ---
Attachment #797766 - Flags: approval-comm-beta?
Attachment #797766 - Flags: approval-comm-aurora?

Updated

5 years ago
Attachment #797766 - Flags: approval-comm-beta?
Attachment #797766 - Flags: approval-comm-beta+
Attachment #797766 - Flags: approval-comm-aurora?
Attachment #797766 - Flags: approval-comm-aurora+
Please make sure this gets landed in time for the final 2.22 beta at the latest.
tracking-seamonkey2.22: --- → ?
(Reporter)

Updated

5 years ago
Assignee: nobody → neil
status-seamonkey2.24: --- → fixed
Flags: needinfo?(neil)
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/releases/comm-aurora/rev/d60fd92ab4cf
https://hg.mozilla.org/releases/comm-beta/rev/1bc86c2043b3
status-seamonkey2.22: affected → fixed
status-seamonkey2.23: affected → fixed
Flags: needinfo?(neil)
(Reporter)

Comment 13

5 years ago
Verified fixed Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 SeaMonkey/2.23a2
Build identifier: 20131019013001.
Status: RESOLVED → VERIFIED
status-seamonkey2.23: fixed → verified
tracking-seamonkey2.22: ? → ---
(Reporter)

Comment 14

5 years ago
Also verified with 2.22 beta 2, Build identifier: 20131023190953.
status-seamonkey2.22: fixed → verified
You need to log in before you can comment on or make changes to this bug.