Close tab button should not be on top of right-most tab at overflow.

RESOLVED FIXED

Status

SeaMonkey
Tabbed Browser
RESOLVED FIXED
15 years ago
9 years ago

People

(Reporter: Jason Bassford, Assigned: jag (Peter Annema))

Tracking

({fixed-seamonkey1.0})

Trunk
fixed-seamonkey1.0
Bug Flags:
blocking1.8b5 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030306
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030306

This is not about dealing with how to access tabs that have overflowed (that's
bug 155325), rather presenting a cleaned up interface when that does happen.

When you open a sufficient number of tabs, the close tab button is drawn on top
of the right-most tab.  What should happen instead, is that the way tabs are
drawn be recalculated so that the space the close tab button takes up is never
impingled upon.  (I.e. make the right margin for tabs what it is now minus the
horizontal space taken up by the close tab icon.)

The drawing of the close tab button on top of an existing tab may not be too
distracting given the default number of tabs shown before overflow, but if that
number is changed via userChrome.css (as per bug 196322) it's readily apparent
and a problem.

Reproducible: Always

Steps to Reproduce:
(Reporter)

Comment 1

15 years ago
Created attachment 116629 [details]
Close tab on top of right-most tab.

Comment 2

15 years ago
The reported behaviour is dependent upon

     tab { min-width: Xem !important; }

in userChrome.css. Without it, I can't duplicate the reported behavior in W32
2003030708 trunk with more than 40 tabs open. With it set to 9em at 1024x768,
the 9th tab is on top of the close tab X. Set to 4em, the 19th tab is on top
(Reporter)

Comment 3

15 years ago
Created attachment 116701 [details]
Same "on top" behaviour w/o userChrome.css - 40 tabs.

Here's a screen shot of a 1280x1024 resolution using 40 tabs (at which point
overflow begins) that shows the same "on top" behaviour.  (Note the "X" just to
the right of the right-most tab's icon, on top of what should be title text,
assuming it were wide enough to show any.)  This is without any custom
userChrome.css but simply using Mozilla's default behaviour.

Comment 4

15 years ago
What theme is that? W32 Modern behaves as I described, but I dont' think it's so
simple. On OS/2 Modern, I don't use the default font style  for tabs (
font-family: warpsans;  font-weight: normal;), and at 1024x768, tab #33
overlaps, without a min-width value set for tab. Looks like the close tab X
needs a Z-index set higher than it is or maybe higher opacity for its background.
(Reporter)

Comment 5

15 years ago
> What theme is that?

Mozilla Modern.  I'm simply using a different Windows theme 
/ style (and a different icon for my throbber) - but my 
Mozilla theme is modern.

If you wish I can revert back to Windows/Luna and change my 
home button to the default, and take a screenshot of that - 
but I know that the same thing would happen there too.  (I 
saw the problem before I switched Windows themes and 
customized my throbber.)
(Reporter)

Comment 6

15 years ago
Created attachment 116710 [details]
Default Modern / Luna in 1024x768, no userChrome.css.

Here's a screenshot of my computer under XP with 2003030608 running in
1024x767, with the default XP Luna theme/style, Mozilla Modern, and none of my
userChrome.css entries in existence.  Overlap still occurs (this time with tab
32).

Comment 7

15 years ago
Fullscreen 800x600 120DPI W32 Modern with css set to 3em puts tab #19 under the X.
(Reporter)

Comment 8

15 years ago
Okay - we could trade examples of number of tabs all day - but the issue here is
that there's always is a point at which a tab appears underneath the X, right? 
It doesn't matter where that point actually is but that it eventually does happen.

This could probably be fixed if the mechanism by which Mozilla is drawing new
tabs were to stop at one tab less than it does now.  The left margin always
behaves correctly, with the left-most tab never impinging upon the open tab
button.  Meanwhile, the right margin *also* behaves properly but only up until
that last tab when overflow occurs.  So, all that needs to be done is for
Mozilla to draw it's (currently calculated maximum number of tabs)-1.
(Reporter)

Comment 9

15 years ago
I just took a look at the latest Phoenix build - it doesn't have this problem. 
The close tab button and the right-most tab are clearly separated, rather than
intermingling.  (However, Phoenix does have a lot of other tabbed browsing
issues that Mozilla, to my mind, does better.)

Comment 10

15 years ago
Definitely a bug, not an enhancement.
Severity: enhancement → normal
Keywords: nsbeta1
(Reporter)

Comment 11

15 years ago
I had never originally intended for this to be an Enhancement bug.  Odd that I
didn't catch this before.

Incidentally, my comment 9 about Firebird was only partially correct.  The Close
Tab button redraws when it has focus (move your mouse cursor over it) so that
there is clear separation between the elements, but when any of the tabs are
focused (click on a tab now) the text of the right-most tab still does appear
"behind" the Close Tab button.  So Firebird is still not totally correct, but it
does do a bit of a better job than Mozilla.

Comment 12

15 years ago
adt: nsbeta1-
Keywords: nsbeta1 → nsbeta1-

Comment 13

14 years ago
Isnt this fixed now?
(Reporter)

Comment 14

14 years ago
Not at all.

Comment 15

14 years ago
*** Bug 226033 has been marked as a duplicate of this bug. ***

Comment 16

13 years ago
Requesting FF 1.0 blocker. Imo, it's too blatant a UI imperfection for a final
release (easily visible for certain types of tabbed browsing) and it seems that
any patch for this should be applicable to FF too.
Flags: blocking-aviary1.0?

Comment 17

13 years ago
*** Bug 259150 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 18

13 years ago
I don't know if this can even, technically, block Firefox since it's not
targeted against that browser.  Even if this bug were fixed, it might apply only
to Mozilla.  I guess it would depend on how "back end" the solution was.

Comment 19

13 years ago
Finally I think you're right. It's better to re-open the related bug and mention
this bug there. 
Flags: blocking-aviary1.0?

Comment 20

13 years ago
I have same problem but different. see next attachment (site loading throbber on
top of close button)

btw noticed this when reproduceing this bug, didn't firefox used to have a right
arrow so you could scroll to the other tabs that are past the end of the
toolbar...or was that removed with the addition of the close tab button...thats
about the last time i seen it.

Comment 21

13 years ago
Created attachment 160391 [details]
site loading throbber is on top of close tab button

Comment 22

13 years ago
sorry just noticed this bug was for Mozilla Browser. should i also file one for
Firefox?

Comment 23

12 years ago
Created attachment 195695 [details] [diff] [review]
Patch: prevents tabs spilling over close button

This just sets overflow:hidden on the hbox containing the tabs, hence any tabs
that would spill out past the close button are now hidden (it is left to bug
155325 to make these accessible).
Attachment #195695 - Flags: review?(neil.parkwaycc.co.uk)

Comment 24

12 years ago
Comment on attachment 195695 [details] [diff] [review]
Patch: prevents tabs spilling over close button

If your intent was solely to hide them, then you should have used
-moz-hidden-unscrollable; as it is I guess your intent us to make it possible
for someone else to develop a patch to scroll them but then you should convert
the hbox into a scrollbox.
Attachment #195695 - Flags: review?(neil.parkwaycc.co.uk) → review-

Comment 25

12 years ago
Created attachment 197079 [details] [diff] [review]
Patch: changed per Neil's comment

Changed hidden to -moz-hidden-unscrollable, as this bug does not justify using
a scrollbox. That should be added (if necessary) if/when scrolling is
implemented. The short term intent behind fixing this bug was to just hide
excess tabs, so in the meantime it at least looks less broken.
Attachment #195695 - Attachment is obsolete: true
Attachment #197079 - Flags: review?(neil.parkwaycc.co.uk)

Updated

12 years ago
Attachment #197079 - Flags: review?(neil.parkwaycc.co.uk) → review?(mconnor)

Updated

12 years ago
Attachment #197079 - Flags: review?(mconnor) → review+

Comment 26

12 years ago
Fix checked in (sr=me).
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Is attachment 197079 [details] [diff] [review] safe for the 1.8 branch?

Comment 28

12 years ago
(In reply to comment #27)
> Is attachment 197079 [details] [diff] [review] safe for the 1.8 branch?

The relevant code is all exactly the same, so it should be. You might want to
ask someone more knowledgeable though.

Comment 29

12 years ago
(In reply to comment #27)
>Is attachment 197079 [details] [diff] [review] [edit] safe for the 1.8 branch?
It's trivial, and looks safe enough to me.

Comment 30

12 years ago
(In reply to comment #29)
>(In reply to comment #27)
>>Is attachment 197079 [details] [diff] [review] [edit] [edit] safe for the 1.8 branch?
Doh, I fell victim to edititis :-(
Asking for 1.8branch blocking to have someone from the 1.8branch look at it and
get it in before the lockdown next week.
Flags: blocking1.8b5?

Comment 32

12 years ago
Comment on attachment 197079 [details] [diff] [review]
Patch: changed per Neil's comment

ask for approval1.8b5 not for blocking1.8b5 since there is a reviewed patched.
Attachment #197079 - Flags: approval1.8b5?
(Reporter)

Comment 33

12 years ago
FWIW: I believe that this fix broke the Flowing Tabs extension.  Upon the normal
reflow, where another row of tabs would be generated, now nothing happens at
all.  I may have 30 tabs open but I'll only ever see 10 (or whatever) at once. 
I'll try to contact the extension author to see if there's some way around this.

Comment 34

12 years ago
So what's the risk of this patch and what areas would need to be tested before
we could be confident that this doesn't break anything?
(Reporter)

Comment 35

12 years ago
I actually had Jomel send me an updated version of Flowing Tabs extension that
he'd re-coded to work with the changes in this bug.  He'd added this to the
extension to get it working again:

.tabbrowser-tabs > hbox {
  overflow: visible !important;
}

But I don't know enough to say what might happen if this were used in CSS
without the extension...
(Reporter)

Comment 36

12 years ago
Okay, I just tested and, as far as I can tell, using that CSS without the
extension still preserves the intent of this bug to keep the close tab on top. 
So modifying the patch to include that would prevent this particular breakage. 
(I haven't encountered anything else.)
(Reporter)

Comment 37

12 years ago
"preserves the intent of this bug to keep the close tab on top"

Visible, and not interfering with anything I should have said. :)

Comment 38

12 years ago
(In reply to comment #34)
> So what's the risk of this patch and what areas would need to be tested before
> we could be confident that this doesn't break anything?

risk: low as it's such a small change. Could break compatibility with Tab Mix
extension, though the extension could easily be updated.
areas: just the tab bar UI. If it works as intended it only prevents overflowing
tabs from covering the close tab button.

(In reply to comment #36)
> Okay, I just tested and, as far as I can tell, using that CSS without the
> extension still preserves the intent of this bug to keep the close tab on top. 

No, quite the contrary! That snippet of CSS completely reverses this bug fix!
All this patch really does is
.tabbrowser-tabs > hbox {
  overflow: -moz-hidden-unscrollable;
}
(except that it's applied inline to the hbox), so that CSS snippet I gave you
reverts the behaviour to the old behaviour needed by Flowing Tabs.

Anyway Flowing Tabs is unmaintained since Fx0.10, even the repackaged version
(which seems to no longer exist) was only compatible up to 1.1, and it'll need
updating anyway for bug 275529, so I'm not too bothered about breaking
compatibility with it. Anyone who wants to keep using it can just use my patched
version (which I'll ask to be added to the Flowing Tabs thread on
extensionsmirror.nl)
(Reporter)

Comment 39

12 years ago
> That snippet of CSS completely reverses this bug fix!

Huh.  I did actually try it, without the extension but with the CSS, and I did
not see the same thing as I saw in my screenshot in comment 2 that led me to
file this bug.  From my own testing, even with that CSS this bug would still be
fixed.

Anyway.  It's still all good.

Comment 40

12 years ago
(In reply to comment #39)
> > That snippet of CSS completely reverses this bug fix!
> 
> Huh.  I did actually try it, without the extension but with the CSS, and I did
> not see the same thing as I saw in my screenshot in comment 2 that led me to
> file this bug.  From my own testing, even with that CSS this bug would still be
> fixed.

Perhaps you're getting caught out by comment 11 again? Firefox was already an
improvement on the Mozilla Suite behaviour in your screenshot... Or else you
might have misapplied the css, it's easy to do. Either way, that css does undo
this bugfix and shouldn't be included (though extensions could do so).
(Reporter)

Comment 41

12 years ago
> Perhaps you're getting caught out by comment 11 again? Firefox was already an
> improvement on the Mozilla Suite behaviour in your screenshot..

Nope. <grin>  Then again, it has been over 2 years since I filed this, so maybe
I should have just resolved this as WORKSFORME some time ago (if that CSS really
does undo this fix, then problem doesn't exist any more, at least in its
original state) - but I haven't *not* been using the Flowing Tabs extension
since then, so I haven't looked at this in a long time.  And I use Seamonkey...

Comment 42

12 years ago
Comment on attachment 197079 [details] [diff] [review]
Patch: changed per Neil's comment

too late for 1.5b
Attachment #197079 - Flags: approval1.8b5? → approval1.8b5-

Updated

12 years ago
Flags: blocking1.8b5? → blocking1.8b5-

Updated

12 years ago
Flags: blocking1.8.1?
Comment on attachment 197079 [details] [diff] [review]
Patch: changed per Neil's comment

First a=me for xpfe part of patch

Comment 44

12 years ago
Comment on attachment 197079 [details] [diff] [review]
Patch: changed per Neil's comment

a=me for SM1.0b on SM only part of code, 2nd needed one - weeee!

Comment 45

12 years ago
SeaMonkey-only portion of patch checked in to the 1.8 branch.
Whiteboard: fixed-seamonkey1.0
Keywords: fixed-seamonkey1.0
Whiteboard: fixed-seamonkey1.0
Clearing blocking1.8.1? since it no longer seems relevant.
Flags: blocking1.8.1?
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.