The default bug view has changed. See this FAQ.

Tabs with long file names triggering tab-overflow with 8 tabs

RESOLVED FIXED in Firefox 15

Status

()

Core
XUL
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jim Jeffery not reading bug-mail 1/2/11, Assigned: jfkthame)

Tracking

({dogfood, regression})

Trunk
mozilla15
dogfood, regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox15- verified)

Details

Attachments

(4 attachments)

STR:
1. Open the following URL's:
http://www.msnbc.msn.com/
http://www.cnn.com/?refresh=1
http://forums.mozillazine.org/viewforum.php?f=23
http://www.neowin.net/
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/
https://tbpl.mozilla.org/
about:memory

2. Open http://www.bing.com/ NOTE: that the Tab-strip overflow is not displayed
3. Close Bing 
4. Open: http://spaceweather.com/ NOTE: now the Tab-strip overflow IS displayed
5. Click the scroll-overflow arrow and notice a blank glass area is displayed where there is normally a tab if its a true tab-overflow.

Broken in cset:
https://hg.mozilla.org/mozilla-central/rev/d871849ac3a3
Good in cset:
https://hg.mozilla.org/mozilla-central/rev/0aa393eef856

Something in the m-i->m-c merge in broken cset is causing this.

Regression , regressession window wanted.
Tested with m-c hourly builds, on Win7 x64 with Medium 125% screen setting 
1680x1050
(Assignee)

Comment 2

5 years ago
Possibly triggered by bug 746421 - any chance you could confirm this using mozilla-inbound or local builds?  I wonder if the tab-strip is looking at visual overflow rects somewhere (rather than frame dimensions) to determine whether things are overflowing.

Comment 3

5 years ago
(In reply to Jonathan Kew from comment #2)
> Possibly triggered by bug 746421
Confirmed from local backout of changes to nsTextBoxFrame.cpp
Blocks: 746421
Component: Tabbed Browser → XUL
Keywords: regressionwindow-wanted
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: tabbed.browser → xptoolkit.widgets
Hardware: x86_64 → All

Updated

5 years ago
Severity: normal → major
tracking-firefox15: --- → ?
I'm assuming that this is also the cause of my tab bar wigging out where it's rapidly switching back and forth between showing arrows and not, especially in GMail.
Keywords: dogfood
Could this also be responsible for the behavior I'm seeing where the tab bar doesn't snap to the right when I close a tab? (Instead, it leaves a huge space until I manually scroll the remaining tabs right.)
(Assignee)

Comment 6

5 years ago
Quite possibly.

It seems like the (mis)behavior is dependent on the length of the title of the rightmost (visible) tab... the tab bar is behaving as though it's using the full width of that tab's title to compute the amount of scroll that may be needed, even though the title is actually truncated to fit within the tab. So if I have a rightmost tab with a title that's only slightly longer than can be displayed, the tab strip scrolls to leave a small blank area at the right; but if my rightmost tab has a really long title, the amount of "overscroll" is correspondingly greater.
(Assignee)

Comment 7

5 years ago
After a little more experimentation - the scrollable extent of the tab bar seems to be based on the "visual overflow" areas that would be computed for *all* the tabs (not just the rightmost one) if they were displaying their full titles. So a long (but truncated) tab title that's somewhere in the middle of the tab bar can still cause an excess scrollable extent, if it's long enough that its full form would project beyond the actual right edge of the last tab.

At this point, it seems to me that this indicates an error in the tab bar (it shouldn't be using untruncated title strings as a basis for computing the scrollable extent, as this doesn't at all reflect the actual size of the tabs). It may be that bug 746421 triggered this by causing more textboxes to report that they have an overflow of some kind, and this is making the tab bar try to take it into account (but it's doing so incorrectly), but I'm not sure bug 746421 by itself would cause these symptoms if the tab bar were using the right basis for its computations.
Created attachment 619426 [details] [diff] [review]
patch

(In reply to Jonathan Kew (:jfkthame) from comment #7)
> At this point, it seems to me that this indicates an error in the tab bar
> (it shouldn't be using untruncated title strings as a basis for computing
> the scrollable extent, as this doesn't at all reflect the actual size of the
> tabs).

The tab strip uses an overflow:hidden frame which it doesn't compute the bounds for. I don't even know what DOM API it would use to do what you describe, or why such an API should exist in the first place.

It seems to me that bug 746421 just incorrectly didn't take cropping into account.
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #619426 - Flags: review?(roc)
(Assignee)

Comment 9

5 years ago
Hmm, I was just about to post the same patch as a possible workaround. Yes, I think bug 746421 should have used the cropped title.

However, I still think there's a deeper issue here; whether the tabs are considered to be scrollable should not depend on the "visual overflow" area but on the "scrollable overflow", which shouldn't have been affected by bug 746421 in the first place.
(In reply to Dão Gottwald [:dao] from comment #8)
> It seems to me that bug 746421 just incorrectly didn't take cropping into
> account.

Neil thought the same in bug 746421 comment 13. However, see also bug 746421 comment 14. So maybe there's more wrong here. My patch fixes the specific issue this bug was filed for, though, as far as I can tell.
(Assignee)

Comment 11

5 years ago
Ah - on looking again, there's a different problem in bug 746421 - it actually stores the "ink bounds" as both visual *and* scrollable overflow, which doesn't seem right. And that would explain the problem here.
(Assignee)

Comment 12

5 years ago
Created attachment 619432 [details] [diff] [review]
patch, distinguish scrollable from visual bounds for nsTextBoxFrame

I think this fixes the overflow calculations in nsTextBoxFrame more correctly. Note that with the visual/scrollable distinction made properly, even using the uncropped title for the visual overflow calculation would no longer cause the tab-strip problem.
Attachment #619432 - Flags: review?(roc)
Comment on attachment 619432 [details] [diff] [review]
patch, distinguish scrollable from visual bounds for nsTextBoxFrame

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

Test please!
Attachment #619432 - Flags: review?(roc) → review+
Attachment #619426 - Flags: review?(roc)

Updated

5 years ago
Duplicate of this bug: 750131
Try build here: 
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-79ac58cd8ad1/ 
based on cset:
https://hg.mozilla.org/try/rev/79ac58cd8ad1

Assuming this is the final r+ patch, it fixed the problem for me.
Severity: major → normal
Status: ASSIGNED → NEW
tracking-firefox15: ? → ---
Damn bugzilla changed the status of the bug because I forgot to 'refresh' the page first...
Severity: normal → major
Status: NEW → ASSIGNED
tracking-firefox15: --- → ?
I am tempted to raise the severity here to blocker, except that the patch here does fix it so I am creating my own builds without this issue.  Otherwise this would be enough to prevent me from using these builds myself.
Comment on attachment 619432 [details] [diff] [review]
patch, distinguish scrollable from visual bounds for nsTextBoxFrame

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

Someone land this :-)
Attachment #619432 - Flags: checkin?
Attachment #619432 - Flags: checkin? → checkin+
https://hg.mozilla.org/mozilla-central/rev/cfaf90b22fc3

Don't forget the tests!
Assignee: dao → jfkthame
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
BTW you don't need 8 tabs for this.  I see it with a few as 4.
(Assignee)

Comment 21

5 years ago
Created attachment 619536 [details] [diff] [review]
test for unwanted tab-strip overflow

Confirmed on try that this fails without the patch, and passes with it.
Attachment #619536 - Flags: review?(roc)
Comment on attachment 619536 [details] [diff] [review]
test for unwanted tab-strip overflow

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

I was thinking of just a reftest, but OK
Attachment #619536 - Flags: review?(roc) → review+
(Assignee)

Comment 23

5 years ago
Created attachment 619544 [details] [diff] [review]
reftest for unwanted scroll-overflow due to cropped textbox label

OK, let's have a reftest as well. I don't really speak XUL, but after a little experimentation, this seems to work to verify that a cropped title doesn't lead to a spurious scroll-overflow area.
Attachment #619544 - Flags: review?(roc)
Attachment #619544 - Flags: review?(roc) → review+
status-firefox15: --- → fixed
tracking-firefox15: ? → -

Updated

5 years ago
Duplicate of this bug: 750481
Tests:

https://hg.mozilla.org/mozilla-central/rev/9498c59bfeb1
https://hg.mozilla.org/mozilla-central/rev/d60f77b10824
Verified on https://tbpl.mozilla.org/?tree=Mozilla-Beta that the tests from Comment 25 passed on Firefox 15 beta(on Linux, OS X and Win):

browser_tabview_bug749658.js
https://tbpl.mozilla.org/php/getParsedLog.php?id=14358623&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=14357799&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=14362739&full=1&branch=mozilla-beta

textbox-overflow-1.xul
https://tbpl.mozilla.org/php/getParsedLog.php?id=14358856&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=14357483&full=1&branch=mozilla-beta
https://tbpl.mozilla.org/php/getParsedLog.php?id=14361744&full=1&branch=mozilla-beta

Based on the above results, setting firefox 15 tracking flag to Verified.
status-firefox15: fixed → verified
You need to log in before you can comment on or make changes to this bug.