407775 - While tabs are open the window area is not drawn correctly on linux.

Status: RESOLVED INVALID

(Core :: Widget: Gtk, defect, P5)

Description

[Ryan Jefferson]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007121008 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b2pre) Gecko/2007121008 Minefield/3.0b2pre

The window area that displays the current page does not have a border when tabs are opened. Native GTK tabs put a border around the open area.

Reproducible: Always

Steps to Reproduce:
1. Start recent build of Firefox.
2. Open tab.
3. Notice drawing issues. 
Actual Results:  
The window area is drawn like an app without tabs open while they are opened. 

Expected Results:  
The area should be drawn like a native GTK app.

Comment 1

[Ryan Jefferson]

Attachment: A comparision of epiphany and firefox.

This is a comparison screenshot between the tabs on epiphany and firefox.

Comment 2

[Martin Stubenschrott]

i think it looks better without that frame, but well, it's probably a personal preference

Comment 3

[Ryan Jefferson]

Withstanding personal preference, it is still an OS integration issue.  If the goal is proper integration the frame should be drawn like Epiphany.  Usability wise the frame gives a better cue to the page selected.  My preference certainly is for the frame, but if GTK drew tabs without the frame I would argue for that as well. 

Comment 5

[Julien "_FrnchFrgg_" RIVAUD]

In bug 412359, Ian posted a "mockup" userchrome, but it suffers from bug 408059. I post it here though as it gives a first idea.

#appcontent tabpanels {
        -moz-appearance: tabpanels !important;
}

#appcontent browser {
        border: 1px solid threedshadow !important;
}

.tabs-bottom {
        border: none !important;
}

.tabbrowser-tab {
        margin-bottom: 0 !important;
}

I'll try to come up with a solution in patch form and not bitten by bug 408059. No guarantees, though, I lack time.

Nota: I think the summary should be changed to "make the page contents be visually into the tabpanel when tabstrip is visible" or "make the browser tabpanels visible when tabstrip is visible", but I didn't change the summary; I'd like your agreement first.

Nota2: An intermediate solution could be to only show the top border of tabpanels, and/or (as in epiphany) draw the page content directly on the tabpanel instead of visually in a widget in the tabpanel (as in attachment 297073 [details]). Tell me which solution you think is more suited.

Nota3: This bug will almost certainly require UI review.

Comment 6

[u294409]

That userchrome didn't made that border in tabbar, but emulated widget inside notebook.

And that userchrome caused drawing issues - sometimes wrong border was shown below the current tab...

Comment 7

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Make browser content be visually in the tabpanels, epiphany look

This is a patch against current trunk, giving Firefox the behaviour of Epiphany. No bugs should still be present (borders appearing when they shouldn't, etc.)

Comment 8

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Make browser content be visually in the tabpanels, epiphany look [v1.1]

Sorry, previous patch contained unrelated modifications to browser.css. This one is correct.

Comment 9

[Tuukka Tolvanen (sp3000)]

fwiw I don't think tabpanel borders look good or serve a purpose around browser content. They look nice when they have padding around them, as they usually do in dialogs; here, the tabpanel borders coincide and overemphasize statusbar and window borders instead of being distinct. They do serve a purpose when separating content inside and outside the tabpanels, as they usually do in dialogs; here, they only separate browser content from the statusbar, which doesn't seem necessary. There could be a positive effect wrt tab head integration to stuff below, but it doesn't seem worth the additional border noise. That's not to say it would necessarily always look horrible or anything, with widget themes having suffiently understated tabpanel borders the result can be acceptable.

Comment 10

[u294409]

It doesn't matter if you like it or not.
It should just look native. There's not place for your personal opinion.

Comment 11

[Julien "_FrnchFrgg_" RIVAUD]

Jakub: We don't have that much examples of tabs containting other things than dialog widgets. There is evolution, and there is the look with an inset border as in attachment 297073 [details]; I'll post a patch with this solution (it's just a matter of adding

.tabbrowser-strip:not([collapsed=true]) + tabpanels > notificationbox {
 -moz-appearance: treeview; /* or listbox */
 margin: 1px
}

at the right place)

I'll also post patches with only the top border of the tabpanels drawn (I'll do this by theming .tabs-bottom and moving it's other borders outside of view). Tuukka, perhaps this will suit you better.

Note that eventually it's the UI drivers that will decide. The patches are easy to do, so I write them to let anyone try the different solutions and choose with enough information.

Comment 12

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Make browser content be visually in the tabpanels, with inner widget [v1]

Here is with an inner widget, looks better for some content bg colors; looks a bit more cluttered, though

Comment 13

[u294409]

Screeny, please.

Comment 14

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Screenshot of previous solutions

Comment 15

[Michael Monreal [:micmon]]

_FrnchFrgg_, great work on the tabs!

My vote: middle. It looks very nice and it's very similar to the tab look in Epiphany. The top one just has too many lines, the bottom one is missing something.

Comment 16

[u294409]

It's the first userchrome.css here or your solution?
The first solution was buggy (eg when tab changed and hovered), I've uploaded a magnified screeny.

Comment 17

[Julien "_FrnchFrgg_" RIVAUD]

Jakub: It's my solution, and it shouldn't be buggy. If you find bugs, tell me.

Comment 18

[Michael Monreal [:micmon]]

_FrnchFrgg_: post people on #tango also seem to prefer the look of the middle variant.

Btw, I think removing the rounded corners at the bottom would look better. Is that possible?

Comment 19

[u294409]

@Michael: remember, it doesn't matter what they/you like.

It must just look native.

Comment 20

[Julien "_FrnchFrgg_" RIVAUD]

Michael: It is part of clearlooks tabs, so I think it's not feasible while keeping native appearance.

Jakub: Yes, it matters what drivers like. It's their program, they get to decide. If you don't like that you can always fork Firefox and fix the bugs yourself. Besides, I am not sure if one can say "it must look native at any price"; there are a lot of other concerns (accessibility, uncluttered UI, Fitz' law, etc...) that could trump "native" appearance, especially in an area (browser tabs) where there isn't a state of the art established way of doing...
It seems, though, that kazehakaze, galeon, and epiphany all use a variant of 2 or 3 (in the tab, without inner widget), mostly without any padding.
The great thing with those solutions is that they go very well with the side bar and look pretty without, too.

I'll post a patch (and screenshots) with only the top border present (that is, almost identical to what is currently done, but with real tabpanel rendering instead of ad-hoc border). I'm not really convinced by this one, but I promised to write a patch for each solution.

Nota: The middle rendering has no patch currently, it's just the epiphany look with a "padding: 1px" rule added at the right spot.

Comment 21

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Only with top border, without additionnal visible widget

Comment 22

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Only with top border, with inner widget

Comment 23

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Screenshot of last two solutions

I hope I didn't goof by asking beltzner an UI review...

Comment 24

[Julien "_FrnchFrgg_" RIVAUD]

Comment on attachment 299058 [details]
Screenshot of previous solutions

Requesting UI review; please tell what solution you want to be implemented.

First three (this screenshot) make the content be visually in the tabpanels, it's better than today when a sidebar is present, and beautiful too when none, but it may be too cluttered. If tabstrip is hidden, the content is directly in the window as before. This solution (without inner widget) has been choosen by several GTK browsers (as evolution and kazehakaze).

The other screenshot presents a solution which is more similar to what is currently done, but with correct native appearance. It's less intrusive, but less easy to the eyes when a sidebar is present, and not similar to what other GTK browsers do. Included is a version with inner widget, for completeness.

Comment 25

[Ian Spence]

I really like the second one, but I was wondering if it ran into any issues
when using tab scrolls (mainly I want to know if the "extension" part of the
selected tab follows through the animation)

Comment 26

[u294409]

"Full with other widget" is the best solution in my opinion.

Comment 27

[u294409]

Only thing I do not like is bold text in current tab...
Probably I must file next bug.

Comment 28

[Julien "_FrnchFrgg_" RIVAUD]

Ian Spence: What do you call a tab scroll ? When tabs are scrolled horizontally in the tabstrip (with the arrows) ? Or scrolling in the content ? In any case, it should work.

Comment 29

[Ian Spence]

horizontally in the tabstrip

Comment 30

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 299213 [details]
Screenshot of last two solutions

Of the two solutions in this screenshot:

top: tabs aren't well associated with content area, background tabs appear broken and entirely disconnected

bottom: a little bit better, but the line extending from the active tab along the top of the content area runs into nothingness on the right side, which looks awkward.

When you submit screenshots, please include the window borders; it's hard to judge without them.

Comment 31

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 299058 [details]
Screenshot of previous solutions

I agree; the middle solution seems to work the best.

Comment 32

[u294409]

Yay for the borders, but height of tabbar is still broken ["jumpy"].

Comment 33

[Tuukka Tolvanen (sp3000)]

which of the preceding patches (if either) corresponds to the middle screenshot in attachment 299058 [details]? does it break Fitt's for the scrollbar of a maximized window?

Comment 34

[Julien "_FrnchFrgg_" RIVAUD]

Attachment: Patch of preferred solution

Comment 35

[Julien "_FrnchFrgg_" RIVAUD]

Tuukka: Attachment 305504 [details] [diff] is what you want (updated to latest trunk if I didn't goof). It will break Fitt's law when the tabstrip is visible AFAIK -- btw my WM already breaks it (maximized windows still have a visible border). That's partly why I proposed solutions with only the top border of tabpanels.

Beltzner: My window borders are completely non standard, I use pekwm with a highly customized theme. I didn't cut anything from the inner window to make the screenshots, though, so you can just imagine a border around them.
Note that the second screenshot (which you ui-r minused) corresponds to a simple refactoring (at least for the "without widget" part) of the current rendering to use the native look of the wanted border instead of a ThreeDShadow line. The rendering is strange only because of the sidebar, exactly as on current trunk.

Comment 36

[Tuukka Tolvanen (sp3000)]

Attachment: with common window borders, before

Comment 37

[Tuukka Tolvanen (sp3000)]

Attachment: with common window borders, after attachment 305504

Comment 38

[Tuukka Tolvanen (sp3000)]

so after building with and using a305504 for enough time to get accustomed to it and then going back to a stock build, I'd say I think I like the changes to the tab bar top, but not the rest: the appearance of extra borders depending on tabs or no tabs is a bit distracting; no add'l borders next to window borders looks cleaner imo; plus the maximized Fitts's bit.

Comment 39

[Julien "_FrnchFrgg_" RIVAUD]

Tuukka: You could try attachment 299210 [details] [diff] [review] (which probably bitrotted a bit).

Comment 40

[Reed Loden [:reed]]

faaborg, what do you think?

Comment 41

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Mockup of sidebar when there is no tabstrip

>faaborg, what do you think?

Overall I think this looks fine, and it addresses beltzner's two concerns of the tab being visually associated with the content area, and the line under the tab connecting on the right side.

My only nit would be the appearance when a sidebar is displayed and there is no tab strip, indicated in the attachment.  If it is possible to add a line here, I think we should.  I looked at the way the sidebar appears in Nautilus with a few different themes, and I guess it doesn't get lines on either side.  So technically my recommendation would make us less native, but I personally think having the line looks a lot better.  Overall it should be up to the linux themers to decide if having that line is worthwhile.

Comment 42

[Julien "_FrnchFrgg_" RIVAUD]

I see two difficulties: choosing the right style for this "line", and knowing when there's a visible sidebar but no tabstrip.

Comment 43

[Julien "_FrnchFrgg_" RIVAUD]

I'd like to have a clear indication of what is wanted eventually, since some said that the currently ui-r+'ed patch  breaks Fitts' law and seem a bit akward when there is no visible sidebar. All in all, I fear that such an UI change could start a flamerebellion that late in the game.

Comment 44

[Julien "_FrnchFrgg_" RIVAUD]

That and we cannot afford any regression in the tab bar now that rc1 is near. The patch shouldn't introduce any, but AFAIK the tab bar is featurefull/complicated enough to be fragile.

Comment 45

[u294409]

Again you're reinventing GTK... Use native components and then talk upstream about fixing compiliance to the Fitt's law.

Breaking such compatibility makes Firefox still look odd in the environment.

First such reinventing was removing padding from toolbars. That change brings nothing to Compiz+GTK-Deco users, even when window maximized.

Comment 46

[Asaf Romano (gone)]

Comment on attachment 305504 [details] [diff] [review]
Patch of preferred solution

I cannot test this, other reviewers can, cancelling request.

Comment 47

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 305504 [details] [diff] [review]
Patch of preferred solution

No longer applies cleanly, unfortunately. Is this bug still an issue?

Comment 48

[u294409]

I tried nightly build. It is still an issue.

Comment 49

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 50

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 51

[Sylvestre Ledru [:Sylvestre]]

No assignee, updating the status.

Comment 52

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1552613

Comment 53

[Zach Cano]

Attachment: same problem

Thanks from solcomptina1971.

Comment 54

[Julien "_FrnchFrgg_" RIVAUD]

This bug is moot since a long time ago Firefox decided to ditch native tabs and prefer a consistent look across platforms. The current look isn't tab based anymore, but uses what looks like buttons.