687836 - Bug-fixing pre-followup patch.

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Blake Winton (:bwinton) (:☕️)]

STR:
Apply the patches for bug 582801, bug 667244, and bug 670304 (in that order).
Build Thunderbird.
See what's still wrong.  ;)

The purpose of this bug is to fix all the little things I find wrong with those other patches, since it seems like a real pain to try and untangle what bits each patch is responsible for, and where each individual fix goes.  None of the patches in those bugs should be checked in until this patch is ready as well, and then we can do them all in one big swoop.

Sound good?

Thanks,
Blake.

Comment 1

[Richard Marti (:Paenglab)]

Now as the QFB button should always be visible, I think it would be better when the button becomes the state "disabled" instead of style="visibility: hidden;". This would made the CSS easier and made it saver the checked state can't be changed in disabled state.

Comment 2

[Blake Winton (:bwinton) (:☕️)]

(I've numbered these so that we can refer to them as, say "Aero-3", or "Mac-5".)

Windows 7 Aero Problems:
  1) There's a white border above the Quick Filter Button.
  2) The QFB seems to always be a button, whereas the Lightning buttons are only button-y when you hover over them.  (Okay, that's probably a problem with Lightning, but we should figure out what the fix is.)
I dragged everything onto the toolbar at this point.
  3) The border under the buttons is lighter than the border under the tabs.
  4) Right-clicking in the tab bar area doesn't let me customize the buttons.
  5) After moving everything off the toolbar, it got much smaller, and fell down behind the tabs.
  6) (Probably unrelated) The "Get Mail" button isn't active in the Gloda Search Results page.
  7) When the QFB is showing, the QF Button glows slightly brighter white, instead of glowing blue like the other buttons do (and like it does when the QFB isn't showing).  (I think it should act like the "Tag" button.)

And that's all I saw on Windows 7.

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Attachment: All the patches from the sub-bugs, rolled up into one easy-to-apply patch.

Hopefully the sub-bugs won't change their patches, and will just build on top of this one instead.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Windows 7 High-Contrast Black (a.k.a. "WinBlack"):
  1) The backgrounds to the buttons are all grey-ish, and make the icons really hard to see.  Ideally I think we'ld skip the gradient in this mode.  (We might want to talk to the Gecko people about how to tell when we're in high-contrast mode...)

I think that's it for High-Contrast.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Just a note that (5) from comment 2 is also appearing on Ubuntu Linux - the toolbar appears to be mostly hidden behind the tabs.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Customizing the toolbar on Ubuntu Linux

Ok, in Ubuntu Linux still:

1) I'm also seeing the white border around the Quick Filter Button, when I'm customizing my toolbar.

2) Also seeing that the border under the buttons, when customizing, is just slightly darker. Screenshot attached.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

Mac.  Mostly good, but:

1) When dragging the buttons off the toolbar, I see the same behaviour as noted in Aero-5, even though the spinner is still on the toolbar.

2) After dragging everything off the toolbar, there's a light grey line at the bottom of the tab bar.  <http://dl.dropbox.com/u/2301433/Screenshots/GreyLine.png>

3) There's a line at the left of the Quick Filter Button.  I would expect that line to be at the end of the toolbar instead.

4) The Quick Filter Button depresses differently than the other buttons in the tab bar.  (Yeah, it's the same as the Tab dropdown, but since we can now have other buttons there I think it should be made similar to them.  Or they should be made similar to it…)

Thanks,
Blake.

Comment 8

[Jim Porter (:squib)]

Aero-1: fixed in the upcoming patch
Aero-2: this will probably be fixed by bug 673990
Aero-3: not totally sure about this yet
Aero-4: this is fairly complicated: the current implementation makes it hard to add a
        context menu to the "background" of the tab bar
Aero-5: this is bug 687730
Aero-6: unrelated; I see this without the patch too (and it will probably be irrelevant
        once we switch to tabs-on-top anyway)
Aero-7: I'll let Richard Marti chime in on this

Linux-1: currently, we use a different image for the QFB when customizing. Maybe we want a
         colorful icon instead?
Lunux-2: maybe we should just do away with the grey background when customizing; I think
         that's what's causing the issue, and Firefox doesn't do that (maybe we should do
         the same for the message header toolbar as well)

Mac-1: bug 687730, again
Mac-2: probably also bug 687730
Mac-3: not sure about this
Mac-4: not sure about this

Comment 9

[Jim Porter (:squib)]

Attachment: Fix some issues with borders

This should fix some issues with borders being screwy (notably the border on the tab bar toolbar and a bogus border on the left of the QFB itself under XP).

Comment 10

[Richard Marti (:Paenglab)]

(In reply to Jim Porter (:squib) from comment #8)
> Lunux-2: maybe we should just do away with the grey background when
> customizing; I think
>          that's what's causing the issue, and Firefox doesn't do that (maybe
> we should do
>          the same for the message header toolbar as well)

Without grey background you don't see any drop target. The whole Firefox tab bar is customizable. On TB only the tabbar-toolbar (a small part of the tab bar). If the tabbar-toolbar during customizing could expand to the last tab could make the need of the grey background obsolete.

Comment 11

[Richard Marti (:Paenglab)]

Attachment: Fix more issues

Patch including squib's fixes.

Aero-3: fixed with this patch
Aero-7: fixed with this patch. The QFB button used a state which is also defined
        for all the other buttons but was hard to trigger. So removed this state
        for the QFB button and let him glow blue.

Mac-3:  removed the border to more look as a part of the tab bar and also as a
        preparation for tabs on top where this borders are looking out of place.
Mac-4:  the QFB button is now acting always like a toolbarbutton (no hover state
        and the same background color when checked).

Comment 12

[Blake Winton (:bwinton) (:☕️)]

Summarizing:

Aero-1: fixed
Aero-2: probably be fixed by bug 673990
Aero-3: fixed
Aero-4: this is fairly complicated: the current implementation makes it hard to add
        a context menu to the "background" of the tab bar
Aero-5: bug 687730
Aero-6: unrelated;
Aero-7: fixed

Mac-1: bug 687730, again
Mac-2: probably also bug 687730
Mac-3: fixed
Mac-4: fixed

Linux-1: currently, we use a different image for the QFB when customizing. Maybe we
         want a colorful icon instead?
Linux-2: maybe we should just do away with the grey background when customizing; I
         think that's what's causing the issue, and Firefox doesn't do that (maybe
         we should do the same for the message header toolbar as well)

So, I think I'ld be okay with landing this if we add a more colourful icon for the QFB for when it's in the customization window and toolbar; do away with the grey background when customizing; and file a new bug for Aero-4.  (Then, after that, hopefully we can land tabs-on-top, and life will be pretty good…  ;)

Paenglab, Squib, which one of you wants to take those last three items, and polish them off?

Thanks,
Blake.

Comment 13

[Richard Marti (:Paenglab)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #12)
> So, I think I'ld be okay with landing this if we add a more colourful icon
> for the QFB for when it's in the customization window and toolbar; do away
> with the grey background when customizing; and file a new bug for Aero-4. 
> (Then, after that, hopefully we can land tabs-on-top, and life will be
> pretty good…  ;)
> 
> Paenglab, Squib, which one of you wants to take those last three items, and
> polish them off?

I can make the first, but I need the colorful icons. Could Andreas create them? And as own images or append to toolbar.png? I'm for the latter.

As I stated in comment 10 without the grey background it would be hard to find the drop targen when the tabbar-toolbar is empty. If you want to remove it anyway, I can do it.

The third should someone do who has better knowledge.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Hey all, just a heads up:

"Aero-4: this is fairly complicated: the current implementation makes it hard to add
        a context menu to the "background" of the tab bar"

I've been able to solve this with my tabs-on-top patch - right clicking on the tabs bar now shows the context menu.

I'm also going to try to make it so that dragging toolbar items onto the tabs bar will cause those toolbar items to be dropped into the new toolbar to the right of the tabs bar.  I'll keep you posted.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

Ok, I've got dropping toolbar items onto the tabs bar working now - it chucks the toolbar items at the head of the tabbar-toolbar toolbar.

This will be available in my patch for bug 644169 once I update it.

Comment 16

[Blake Winton (:bwinton) (:☕️)]

Since Mike got the context menu and toolbar item dropping working, I think we can remove the grey background.  :)

Andreas should be able to create the images (from comment 13) pretty quickly.

And then we can check this in, along with the bugs it depends on…  :)

Comment 17

[Andreas Nilsson (:andreasn)]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #16)
> Since Mike got the context menu and toolbar item dropping working, I think
> we can remove the grey background.  :)
> 
> Andreas should be able to create the images (from comment 13) pretty quickly.

Coming right up!

Comment 18

[Andreas Nilsson (:andreasn)]

Attachment: graphics part of the icons

Here are the graphics for XP, Aero and Pinstripe. For Linux I think we can use the search icon from GTK+
Will put up a more proper patch once I've got all the other patches this bug depends on running properly.

Comment 19

[Andreas Nilsson (:andreasn)]

Attachment: graphics part of the icons part2

Richard noted a bit of overlap in mail-toolbar-small.png. Fixes that.

Comment 20

[Richard Marti (:Paenglab)]

Attachment: Patch with colored QFB icons

This patch removes the grey background on tabbar-toolbar in customizing mode. I'm using now always the colored icons from Andreas or system icons under Linux. I think it's the best always use this icons because the QFB button is now like a normal toolbarbutton and is placeable everywhere on the toolbars and should look always the same.

I'm asking r and ui-r because now all issues should be solved and Blake said he wants to land shortly.

Comment 21

[Richard Marti (:Paenglab)]

Attachment: QFB button on Mac

One thing I'm not sure is the Mac search icon. For me it looks a little bit taller (or a little shifted up). What do you mean, I took it between two of the tallest icons?

Comment 22

[Blake Winton (:bwinton) (:☕️)]

Yeah, it's definitely a pixel or two taller than the other icons.

Andreas, did you want to shrink it down a tiny bit?

Comment 23

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 570001 [details] [diff] [review]
Patch with colored QFB icons

Review of attachment 570001 [details] [diff] [review]:
-----------------------------------------------------------------

I don't really like the way the QFB icon changes on OS X (as demonstrated here: http://dl.dropbox.com/u/2301433/Screenshots/SpyglassStates.png ), particularly having the huge white glass bit disappear on hover feels really odd to me, and different from anything else.  Also, it looks like we're cutting off the bottom of the shadow on the QFB icon.  So I'm going to say ui-r-.

Aside from that, the code seems pretty good, so r=me.

Comment 24

[Richard Marti (:Paenglab)]

Some update about the QFB icon under OSX?

Comment 25

[Mike Conley (:mconley) (:⚙️)]

Attachment: Unbitrotted patch with colored QFB icons -[r+ and ui-r- by bwinton]

I hope I'm not uploading out of turn here - bug 644169 needed this un-bitrotted, so I took the liberty of doing that.

Nothing too major - but maybe go through it once to make sure I didn't do something silly.

Comment 26

[Andreas Nilsson (:andreasn)]

Attachment: updated version of the patch above

This adds makes it :hover:active instead of :hover in quickFilterBar.css, so that the icons goes dark on downpress.
Also removes the shadow on the search icon.

Comment 27

[Andreas Nilsson (:andreasn)]

Attachment: graphical glitch with search button

The search icon seems to become a archive when any other tab than the main one is selected. Not quite sure why that is happening yet.

Comment 28

[Richard Marti (:Paenglab)]

Attachment: Next try for review

This patch fixes the glitch on inactive QFB button on Mac (wrong -moz-image-region definition).

I'm asking only for ui-r because since the r+ only the unbitrot and my glitch fix have changed. But feel free if you want change it.

Comment 29

[Richard Marti (:Paenglab)]

Attachment: Again a try for review

I've found the unbitrot patch corrupted the files mainwindow-dropdown-arrow-inverted.png and tab-arrow-left-inverted.png. This patch corrects this.

Comment 30

[Andreas Nilsson (:andreasn)]

Works much better now!
A small thing I noted when dragging it around was that I don't seem get any icon for Search in the customize toolbar window.

Comment 31

[Andreas Nilsson (:andreasn)]

and this seems to be because it get visibility: hidden; added from somewhere.

Comment 32

[Richard Marti (:Paenglab)]

Attachment: Last try for review?

I could reproduce the inactive icon state. This happens only when the QFB button is moved to the customize window on a tab where the button is in disabled state. Now the button is always visible in customize window.

Comment 33

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 576611 [details] [diff] [review]
Last try for review?

Review of attachment 576611 [details] [diff] [review]:
-----------------------------------------------------------------

So, it still looks really odd to me, and so I've spent some time thinking about why.

1) While all the other toolbar buttons go dark on click, they all have more blocky shapes, and are multi-coloured to start off with, so the darkening looks much better than the spyglass where only the circle in the middle is changed.

2) In Firefox, when you click the monochrome buttons in the toolbar either the shadow underneath them gets darker, or they glow blue, and this is the direction I'ld like to see us head.

But this set of patches has been lingering for far too long, so I think I'm okay with landing them, and doing the rest of the work in a follow-up bug.

Thanks,
Blake.

Comment 34

[Richard Marti (:Paenglab)]

Asking checkin-needed so the follow-up bugs can be filed ;).

Comment 35

[Mike Conley (:mconley) (:⚙️)]

Checked in to comm-central as http://hg.mozilla.org/comm-central/rev/24a39254d6a8