Bug 541052 (ToolboxMaxHeight)

Toolbox does not expand vertically anymore

RESOLVED FIXED

Status

--
major
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: realRaven, Assigned: bwinton)

Tracking

({regression})

unspecified
x86
Windows XP
regression

Firefox Tracking Flags

(blocking-thunderbird3.0 .2+, thunderbird3.0 .2-fixed)

Details

(Whiteboard: [patch up, needs patch to add tests.])

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.7) Gecko/20100111 Lightning/1.0b1 Thunderbird/3.0.1

Some change in  TB 3.0.1 prohibit vertical expansion of toolbars. The Extension QuickFolders creates a new toolbar on which folders can be dragged; this creates toolbarbuttons that look like tabs - they wrap to the next line (max-height is set to 25em) expanding the toolbar vertically. Since the upgrade to 3.0.1 the toolbar does not expand (vertically) anymore. The box containing the folders has these rules:
#QuickFolders-FoldersBox { display:  -moz-inline-box; float:right; max-height:25em; }
in the XUL file it is:
<box id="QuickFolders-FoldersBox" flex="10">
tried using various themes, same effect.


Reproducible: Always

Steps to Reproduce:
1. Install the QuickFolders extension
2. Drag Folders (from the folder tree) onto the toolbar, causing the folders to wrap to the next line
3. Make Thunderbird narrower so that right-most folder buttons will be hidden.
Actual Results:  
Folder Tabs to the right disappear

Expected Results:  
Folder Tabs wrap to the next line and the toolbar increases in height.
(Reporter)

Comment 1

9 years ago
Using the DOM inspector, I found out that if you have multiple (user defined) toolbars e.g. Tag Toolbar there are pushed so far down the mail-toolbox, that they become invisible. This can be easily tested by clicking on the toolbars in the DOM-Node Tree while the "Blink selected Elements" option is enabled. So this is likely going to affect a lot of extensions that create toolbars which are displayed within the mail-toolbox.`As a workaround, I have changed overflow-y from hidden to visible, (so I can scroll into the toolbars) but that doesn't change the height restriction.
(Reporter)

Comment 2

9 years ago
The bug can be worked around by adding this rule set to userchrome.css:
#mail-toolbox { overflow-x:visible !important; overflow-y:visible !important;}

the "offending rules" of setting overflows to hidden are in 
chrome://messenger/content/messenger.css
Blocks: 536245
blocking-thunderbird3.0: --- → ?
Keywords: regression
(Reporter)

Updated

9 years ago
Alias: ToolboxMaxHeight
Summary: Toolbars do not expand vertically anymore → Toolbox does not expand vertically anymore
Can you do a patch or diff of that?
Let me know if you need help setting up a build environment (it's so easy even I managed to do it :).
(Reporter)

Comment 4

9 years ago
Shouldn't the Fix from Bug 536245 rolled back for this? I am only an extension editor at the moment, so I haven't ever built TB.
(In reply to comment #4)
> Shouldn't the Fix from Bug 536245 rolled back for this? I am only an extension
> editor at the moment, so I haven't ever built TB.

Not necessarily. AFAIK a fix was suggested on the newsgroups that would mean that bug 536245 can stay in and toolbars still work for your extension.
blocking-thunderbird3.0: ? → .2+
(Reporter)

Comment 6

9 years ago
IMHO - the original bug proposed to make overflow hidden for the
toolbox container - causing some bad side effects for user created toolbars. 

Instead, we should only hide the overflow for the actual
(first) toolbar (mail-bar3); if I add these rules, I can fix both issues, from Bug 541052 and Bug 536245. Note that overflow-y does not need to be specified.

#mail-toolbox { /* this part merely undoes the rules from Bug 536245 */
  overflow-x:visible !important;
  overflow-y:visible !important;
}
#mail-bar3 {
    overflow-x:hidden !important;
}
I will add an attachment with a screenshot.
(Reporter)

Comment 7

9 years ago
Created attachment 423003 [details]
TB after fix

For this test, I have added buttons to the toolbar and switched it to "Icons
beside Text".
After applying the userChrome rules from my previous comment:

1. the mail toolbar is cut off at the right, as expected
2. the quickfolders toolbar can expand (as designed) by floating its buttons
down
3. the other screen elements are not cut off (e.g. scrollbars of message pane
are visible)

Can somebody more versed in patching TB create a patch? I believe the overflow
rules that were added in Bug 536245 for #mail-toolbox have to be removed.

Comment 8

9 years ago
> Created an attachment (id=423003)
> TB after fix
Aaargh! My eyes hurt!
(Reporter)

Comment 9

9 years ago
ROFL! 8P

I just like Plashtic (Mostly Crystal)! I know that example was a bit extreme ... teehee... but you get the point ;}
Assignee: nobody → dmose
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 10

9 years ago
Created attachment 426731 [details] [diff] [review]
A patch to change the overflow rules for the toolbar.

It would be really nice if we could drive this in by Monday's code freeze.  (And nicer still if I didn't have to work on that Monday, cause it's Family Day. :)

Screenshot coming up.
Assignee: dmose → bwinton
Status: NEW → ASSIGNED
Attachment #426731 - Flags: ui-review?(clarkbw)
Attachment #426731 - Flags: review?(philringnalda)
(Assignee)

Comment 11

9 years ago
Created attachment 426732 [details]
A screenshot of the previous patch.

There you go, Bryan.  Lemme know what you think.
First, let me state that while I asked you to look at this bug (and very much appreciate that you have), I'm also very unwilling to ask you to spend your own weekend or holiday time on it, and I think you shouldn't do it (same goes from Bryan regarding ui-review).  (It's not like this release is a security firedrill, after all).  

I'm going to steal the review from philor, and mark this r+, since I have no idea whether he's likely to see this or be around this weekend.  I really dislike that we haven't had time to write a test here, but I think shipping without one is slightly better than slipping to 3.0.3.

Bryan, if you get time to offer ui-r today and decide on ui-r+, please add the checkin-needed keyword to this bug (for the reader following along at home, Bryan's out all of next week, so today will be the last chance).  Since Standard8 is gone for the weekend and out on Monday, a=dmose for landing on both trunk and branch assuming ui-r+ is granted.

All that said, I think slipping this fix to 3.0.3 wouldn't be the end of the world.  If it doesn't make it, life will go on.
Attachment #426731 - Flags: review?(philringnalda) → review+
(Reporter)

Comment 13

9 years ago
Hi Blake, thanks for looking at this on a Saturday!
Sorry that patch doesn't work it causes a scrollbar to appear - for some reason the toolbar area does not expand vertically for this.

But  what about this:
#mail-toolbox {
  overflow-x:visible !important;
  overflow-y:visible !important;
}
#mail-bar3 {
  overflow-x:hidden !important;
}

This will restrict the mail-bar3 width without impacting on the toolbox as a whole! I don't know how to turn this into a patch [I am only writing extensions, so I currently haven't got the know how to build TB myself], but if you like me to test against the original bug to see that it cures the original problem please give me a shout.
(Reporter)

Comment 14

9 years ago
Created attachment 426812 [details]
Using #mail-bar3 {overflow-x:hidden !important;}

screen behavior after applying the rules from my previous comment.
It also seems to fix the behavior of  Bug 536245. Of course there might be other build in toolbars in TB3 that need the same rule as

#mail-bar3 {overflow-x:hidden !important;}

It also is important to keep 
#mail-toolbox { overflow-x:visible; overflow-y:visible;}

because the Users won't like the scrollbar - expectation is that the any toolbar areas (Menus / tab bars) contain usable controls are always visible.
(Reporter)

Comment 15

9 years ago
(In reply to comment #14)
There is a typo in the screen shot, it should be 
"cropping starts from the right"
and not
"cropping starts right to left".
That obviously doesn't make sense.
Oh and yes, I would love if somebody could turn that into a patch, if it doesn't go into 3.0.3 that's no major issue, as long as I can tell the users it is going to be fixed eventually.

In the meantime, is it possible (and desirable) to include these css rules in my extension's css files, until the fix made its way into the trunk?
(Assignee)

Comment 16

9 years ago
Created attachment 426829 [details] [diff] [review]
[checked-in] A patch that's more like Axel's.

(In reply to comment #14)
> Of course there might be
> other build in toolbars in TB3 that need the same rule as
> #mail-bar3 {overflow-x:hidden !important;}

Yeah, it's the other toolbars that may need this that's my main problem with the patch.

Having said that, I tried it with the QuickFolders, and with a custom toobar, and couldn't see anything wrong, so maybe it won't be too bad.  And we can suggest to extension developers that they add a similar rule if we run into problems.

> It also is important to keep 
> #mail-toolbox { overflow-x:visible; overflow-y:visible;}
> because the Users won't like the scrollbar - expectation is that the any
> toolbar areas (Menus / tab bars) contain usable controls are always visible.

Yeah, I agree, but as it turns out, that's the default, so we don't need to do anything other than revert the previous change if we want this in core.

Later,
Blake.
Attachment #426812 - Attachment is obsolete: true
Attachment #426829 - Flags: ui-review?(clarkbw)
Attachment #426829 - Flags: review?(dmose)
(Assignee)

Updated

9 years ago
Attachment #426812 - Attachment is obsolete: false
(Assignee)

Updated

9 years ago
Attachment #426731 - Attachment is obsolete: true
Attachment #426731 - Flags: ui-review?(clarkbw)
Attachment #426829 - Flags: ui-review?(clarkbw) → ui-review+
Comment on attachment 426829 [details] [diff] [review]
[checked-in] A patch that's more like Axel's.

looks good to me
Comment on attachment 426829 [details] [diff] [review]
[checked-in] A patch that's more like Axel's.

r=dmose, conditional on adding some wiki-content to MDC so that authors have some source of info to know about what they should be doing with their toolbars.
Attachment #426829 - Flags: review?(dmose) → review+
Attachment #426829 - Flags: approval-thunderbird3.0.2+
(Assignee)

Comment 19

9 years ago
Pushed to comm-1.9.1 as http://hg.mozilla.org/releases/comm-1.9.1/rev/d291fafce578

(And I'm waiting until comm-central is a little less burny to push there.)
(Assignee)

Comment 20

9 years ago
And finally pushed to comm-central as
http://hg.mozilla.org/comm-central/rev/97a3ccf97728
Whiteboard: [patch up, needs patch to add tests.]
(Assignee)

Updated

9 years ago
Attachment #426829 - Attachment description: A patch that's more like Axel's. → [checked-in] A patch that's more like Axel's.
status-thunderbird3.0: --- → .2-fixed
(Reporter)

Comment 21

9 years ago
Fixed, as of Thunderbird 3.0.3 :)
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.