Closed Bug 850924 Opened 11 years ago Closed 11 years ago

Australis tabs don't leave overflow mode when they should

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: MattN, Assigned: Gijs)

References

Details

(Keywords: regression, Whiteboard: [Australis:M3])

(Quoting Valerio from bug 738491 comment #134)
> (In reply to Guillaume C. [:ge3k0s] from comment #133)
> > (In reply to Guillaume C. [:ge3k0s] from comment #128)
> > > Don't know if it's related to the new patch but there is a new bug in UX
> > > builds with the close button doesn't making the tab close (except for the
> > > farthest tab on the right). At first glance it seems plugin-related, but I
> > > don't know how to reproduce it all the time.
> > 
> > Never mind I've tried several times to reproduce this issue and wasn't able
> > to do it. If it happens again I'll post here.
> > 
> > 
> > (In reply to Valerio from comment #132)
> > > Created attachment 723095 [details]
> > > overflow's arrows remains visible
> > > 
> > > If I open many tabs in overflow mode and then I close them, the left and
> > > right arrows of the overflow mode remains visible
> > 
> > I can't reproduce this. I had the same problem when UX branch was busted,
> > but it doesn't happen to me with latest build. Do you have a clearer way to
> > reproduce it ?
> 
> Step to reproduce:
> - open UX;
> - open up many tabs with the new tab button to activate overflow mode;
> - right click on the first tab(the homepage tab in the screenshot that I
> previously posted here) and select close other tabs

(Quoting fx4waldi from bug 738491 comment #135)
> Another method:
> - Open UX
> - Open a few tabs in a maximized window
> - Minimize window
> - Set the width of the window to activate overflow mode
> - Maximize window

I'm not sure which Australis patch caused the issue.
Can you still reproduce this?
Some tabbed browsing code changed recently that might have fixed this.
Also, it might be due to not setting the correct padding-start/-end on the inner scrollbox to offset margin-start/-end on the first/last tab.
This is reproducible on last night's UX nightly with Windows 7.  Thanks for the pointers I suspect it's the latter may be from switching to %define for the margin/padding on the ends of the tabs.
In windows 7 I I noticed that happens only using the aero glass theme.
Using the aero basic theme and the classic theme it seems that the problem is not present
Whiteboard: [Australis:M3]
The platform/OS stuff is marked as all/all here, but I can't reproduce on OSX with latest nightly, and comment #3 seems to indicate this is Windows 7 Glass specific? Will test there in a sec.
OK, I can reproduce on Win7, taking.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
This broke between UX nightlies of 3/14 and 3/16, AFAICT. I don't know why yet, and the hg revision links in the nightlies no longer work because the UX branch got reset, so I can't get a proper regression range or anything like that. :-(

I do suspect Matt's comment #0 is on the mark and it has to do with the define patch, but I don't see how that could break this. I did notice that it *seems* that before the patch, the half-curve width stuff on windows was 14px, and it was unified (cross platform) to 15px. However, I'm not 100% sure because I can't see actual revision logs and diffs, only the patches on the bug and speculating what was on ux at the time. Furthermore, I haven't yet found what relies on that nor verified that changing it back to 14px for Windows would fix the issue. Just fiddling with the padding on the scrollbox doesn't seem to make any difference.

Because I don't have a windows build env, and the actual underflow triggers come from compiled code (http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp#2569), this is hard to tackle. Matt, given this info, do you have an offhand clue where I should look? Otherwise, if we want to have this fixed by Thursday, someone else should probably look at it.
Flags: needinfo?(mnoorenberghe+bmo)
Perhaps there is a border/margin/padding/pseudo-element that is not being taken into account in the CSS making the 15px not quite right. Finding the problem with only the minimal patches may help narrow the problem down. The M3 deadline isn't until May 1 but if you don't have a Windows build env. it would make it hard to tweak things like pseudo-elements and make testing a patch quite hard. If you want to hand it over to someone else, that's fine given the situation.
Frank would have more suggestions about what caused this problem in the past.
Flags: needinfo?(mnoorenberghe+bmo)
We discussed this on IRC and Mike said he could take a look. Thanks!
Assignee: gijskruitbosch+bugs → mdeboer
This is where my investigation lead me: tracing back to the layout stack in C++ it appears that the underflow/ overflow events are not fired for the tabstrip in the reproducable scenario.
It appears that when tabs are added in a maximized window, the tabs are drawn inside the titlebar area. When you resize the browser to a size where an overflow is triggered and the tab scrollers appear, the tabs will be drawn beneath the titlebar area. When you maximize the browser again, causing the tabs to be drawn inside the titlebar again, no underflow event is fired, thus suggesting that the scrollbox doesn't keep track of its updated dimensions when drawn inside the titlebar.

I haven't had the time to come up with an actual patch this week, due to work on other bugs and the considerable amount of time it took to investigate this and come up with the aforementioned case. I'm leaving this info here for anyone who has the time to work on this until coming Monday.
(In reply to Mike de Boer [:mikedeboer] from comment #9)
> This is where my investigation lead me: tracing back to the layout stack in
> C++ it appears that the underflow/ overflow events are not fired for the
> tabstrip in the reproducable scenario.
> It appears that when tabs are added in a maximized window, the tabs are
> drawn inside the titlebar area. When you resize the browser to a size where
> an overflow is triggered and the tab scrollers appear, the tabs will be
> drawn beneath the titlebar area. When you maximize the browser again,
> causing the tabs to be drawn inside the titlebar again, no underflow event
> is fired, thus suggesting that the scrollbox doesn't keep track of its
> updated dimensions when drawn inside the titlebar.
> 
> I haven't had the time to come up with an actual patch this week, due to
> work on other bugs and the considerable amount of time it took to
> investigate this and come up with the aforementioned case. I'm leaving this
> info here for anyone who has the time to work on this until coming Monday.

I can actually also reproduce this with the first set of STR:

(Quoting Valerio from bug 738491 comment #134)
> Step to reproduce:
> - open UX;
> - open up many tabs with the new tab button to activate overflow mode;
> - right click on the first tab(the homepage tab in the screenshot that I
> previously posted here) and select close other tabs

Which shouldn't have anything to do with whether or not we're drawing in the titlebar...
Assignee: mdeboer → nobody
Status: ASSIGNED → NEW
I didn't get very far with this - getting the feeling I'm searching for a needle in a haystack... in the dark :) I'm missing too much context, wasting too many cycles on it to efficiently reach a conclusion. Paired-coding would help.

Until then I'll leave this bug unassigned. Feel free to take it.
So I bisected this locally, and it's caused by this changeset: http://hg.mozilla.org/projects/ux/rev/9c493366a3da (bug 813802).

I'll try and isolate this further, but I don't know if I'll have time today and I'm not around my Windows machine again until Monday. If someone else wants to take it before then, feel welcome! :-)
Depends on: 813802
Keywords: regression
Assignee: nobody → mconley
Gijs is being awesome and taking this off my lap.
Assignee: mconley → gijskruitbosch+bugs
So I literally just figured out it's related to the placeholders, which get hidden if the [tabsintitlebar] attribute gets set. Then I thought "hey, so maybe the removal of the app button fixes it". On a fresh build off jamun tip, I don't see this anymore. :-)

(I'm still not entirely sure why the app menu placeholder was more problematic than the caption button one - maybe because there's something funny going on with ordinal there? - but hey...)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to :Gijs Kruitbosch from comment #14)
> So I literally just figured out it's related to the placeholders, which get
> hidden if the [tabsintitlebar] attribute gets set.

That should be "... if [it] *doesn't* get set", obviously...
You need to log in before you can comment on or make changes to this bug.