Last Comment Bug 475327 - [Linux] New Tab button is still right side of the Tab bar
: [Linux] New Tab button is still right side of the Tab bar
Status: VERIFIED FIXED
: verified1.9.1
Product: Firefox
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: 3.5 Branch
: x86 Linux
: P2 normal with 1 vote (vote)
: Firefox 3.6a1
Assigned To: Dão Gottwald [:dao]
:
: Dão Gottwald [:dao]
Mentors:
Depends on:
Blocks: 457651
  Show dependency treegraph
 
Reported: 2009-01-26 04:12 PST by Hideo Oshima
Modified: 2009-11-12 16:44 PST (History)
17 users (show)
mbeltzner: blocking‑firefox3.5+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screnshot of Tab bar (52.97 KB, image/png)
2009-01-26 04:12 PST, Hideo Oshima
no flags Details
patch (9.11 KB, patch)
2009-01-26 11:33 PST, Dão Gottwald [:dao]
gavin.sharp: review+
Details | Diff | Splinter Review

Description Hideo Oshima 2009-01-26 04:12:32 PST
Created attachment 358822 [details]
Screnshot of Tab bar

Bug 457651 has been fixed but New Tab button is still right side of
the Tab bar with Linux build.

Please see the screenshot.
Comment 1 ojab 2009-01-26 04:55:35 PST
The same (New Tab button on left) on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090125 Minefield/3.2a1pre
Comment 2 Dão Gottwald [:dao] 2009-01-26 05:16:25 PST
Looks like we're getting a flawed overflow event from the arrowscrollbox.
Comment 3 Dave Garrett 2009-01-26 05:54:50 PST
I only see this using the default theme. Other themes show with it at the end of the tab list.
Comment 4 Ed Lee :Mardak 2009-01-26 09:11:22 PST
I was playing around with the styles on os x and accidentally changed the margin-bottom of the tabs. This seems to have caused this same bug, so it's overflowing vertically and thinking it should show the right-aligned new tab button.
Comment 5 Dão Gottwald [:dao] 2009-01-26 11:33:59 PST
Created attachment 358879 [details] [diff] [review]
patch

Yeah, we need to check the overflow direction.
I think it's also time to clean up the handling of these events a bit.
Comment 6 Dão Gottwald [:dao] 2009-01-26 12:03:49 PST
Comment on attachment 358879 [details] [diff] [review]
patch

>diff --git a/toolkit/content/widgets/scrollbox.xml b/toolkit/content/widgets/scrollbox.xml

>-      <handler event="underflow"><![CDATA[
>+      <handler event="underflow" phase="capturing"><![CDATA[

>-      <handler event="overflow"><![CDATA[
>+      <handler event="overflow" phase="capturing"><![CDATA[

Note that these handlers need to run before those in tabbrowser.xml, hence this change.
Comment 7 Hideo Oshima 2009-01-27 05:40:10 PST
(In reply to comment #5)
> Created an attachment (id=358879) [details]
> patch
> 
> Yeah, we need to check the overflow direction.
> I think it's also time to clean up the handling of these events a bit.

I tried the patch.
It looks good for me.
Thanks.
Comment 8 Michael Kohler [:mkohler] 2009-01-29 09:41:29 PST
Works for me in Ubuntu 8.10 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090129 Minefield/3.2a1pre ID:20090129020428 ...

The patch isn't implemented yet, is it?
Comment 9 Dão Gottwald [:dao] 2009-01-29 09:47:58 PST
No, it isn't.
Comment 10 Michael Kohler [:mkohler] 2009-01-29 13:12:01 PST
So that means that it is fixed without this patch? Or am I the only one?
Comment 11 Dão Gottwald [:dao] 2009-01-29 13:19:54 PST
To reproduce this bug, you need to use the default theme and open a window where the tab bar doesn't overflow immediately (that is, without restoring a session with lots of tabs).
Comment 12 Michael Kohler [:mkohler] 2009-01-29 13:24:05 PST
Oh, I'm sorry.

But it occurs also when I use a new profile and then use the normal one.
Comment 13 Michael Kohler [:mkohler] 2009-01-29 16:06:16 PST
You can download my build (just finished compiling) at http://mozilla.webdesign-portal.net .. Looks like the patch works!
Comment 14 Michael Kohler [:mkohler] 2009-02-05 05:27:31 PST
Sometimes it works for me without this patch.. But only sometimes and I couldn't figure out why it does that..
Comment 15 :Gavin Sharp [email: gavin@gavinsharp.com] 2009-02-11 12:32:19 PST
Comment on attachment 358879 [details] [diff] [review]
patch

(In reply to comment #6)
> Note that these handlers need to run before those in tabbrowser.xml, hence this
> change.

Why? r=me with that explained.
Comment 16 Dão Gottwald [:dao] 2009-02-11 12:41:37 PST
(In reply to comment #15)
> (From update of attachment 358879 [details] [diff] [review])
> (In reply to comment #6)
> > Note that these handlers need to run before those in tabbrowser.xml, hence this
> > change.
> 
> Why? r=me with that explained.

Because this.scrollBoxObject.ensureElementIsVisible(tabs.selectedItem); needs to run after the scroll buttons are shown, otherwise the selected tab won't be fully visible. This means that the underflow handler in scrollbox.xml doesn't need to change, but I figured it's better to make both cases consistent.
Comment 18 Marcia Knous [:marcia - use ni] 2009-02-13 11:23:38 PST
Verified on the 1.9.1 branch using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090213 Shiretoko/3.1b3pre.
Comment 19 Henrik Skupin (:whimboo) 2009-11-12 16:44:42 PST
Verified fixed with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b3pre) Gecko/20091111 Namoroka/3.6b3pre ID:20091111033750

Note You need to log in before you can comment on or make changes to this bug.