Last Comment Bug 585946 - Location / search bar splitter moves to the end of the toolbar when toggling "tabs on top"
: Location / search bar splitter moves to the end of the toolbar when toggling ...
Status: RESOLVED FIXED
: polish, regression
Product: Firefox
Classification: Client Software
Component: Toolbars and Customization (show other bugs)
: Trunk
: x86 Windows 7
: -- minor with 3 votes (vote)
: Firefox 14
Assigned To: Dão Gottwald [:dao]
:
Mentors:
: 586158 589451 624113 642115 (view as bug list)
Depends on: 740974
Blocks: 555081
  Show dependency treegraph
 
Reported: 2010-08-10 07:42 PDT by Theodore
Modified: 2012-03-30 14:39 PDT (History)
17 users (show)
dao+bmo: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.x+


Attachments
patch (4.93 KB, patch)
2010-11-11 05:16 PST, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
better patch (1.94 KB, patch)
2011-02-20 14:14 PST, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
test added (4.40 KB, patch)
2011-02-23 05:40 PST, Dão Gottwald [:dao]
no flags Details | Diff | Splinter Review
patch (4.81 KB, patch)
2012-03-21 08:22 PDT, Dão Gottwald [:dao]
neil: review+
Details | Diff | Splinter Review

Description Theodore 2010-08-10 07:42:24 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b4pre) Gecko/20100810 Minefield/4.0b4pre

The search field/location bar cannot be resized if you set tabs to be on bottom.

Reproducible: Always

Steps to Reproduce:
1. When tabs are on top, hover mouse over the divider between the location bar and search field, notice that arrows appear and the bar is resizable.
2. Right-click toolbar and uncheck "Tabs on Top" option.
3. Hover mouse over location bar/search field divider again, notice that no arrows appear and the bar is not resizable. 
Actual Results:  
Location bar/search field is not resizable.

Expected Results:  
Should be able to resize search field.

I tested this with Aero enabled. If you switch the tabs back on top, the bar is still not resizable. It only becomes resizable again if you restart the browser or right-click toolbar, choose customize, and click "Restore Default Set".
Comment 1 Alice0775 White 2010-08-10 09:14:38 PDT
I can confirm in Aero style.
The problem does not happen in non-Aero style.

After having carried out "Customize toolbar > Done", the rersizer works again.


Regression window:
Works:
Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100623 Minefield/3.7a6pre ID:20100624095420
Fails:
http://hg.mozilla.org/mozilla-central/rev/cc7c030f6b42
Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a6pre) Gecko/20100623 Minefield/3.7a6pre ID:20100624112154
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4a6f3d15cfdc&tochange=cc7c030f6b42
Candidate Bug:
Bug 555081 - Can't move the window using the titlebar with custom titlebar drawing or glass areas below the titlebar
Comment 2 XtC4UaLL [:xtc4uall] 2010-08-11 06:41:42 PDT
*** Bug 586158 has been marked as a duplicate of this bug. ***
Comment 3 XtC4UaLL [:xtc4uall] 2010-08-11 06:42:50 PDT
Seems to be Win7 only, right?
Comment 4 XtC4UaLL [:xtc4uall] 2010-08-21 09:18:31 PDT
*** Bug 589451 has been marked as a duplicate of this bug. ***
Comment 5 :Felipe Gomes (needinfo me!) 2010-08-24 10:55:00 PDT
Likely not a widget bug. This code fails to update correctly for some reason:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#2366

Maybe this is something similar to bug 555987 [{inc}Dynamically changing -moz-box-ordinal-group doesn't work].
Comment 6 Alice0775 White 2010-08-24 11:27:40 PDT
Changing tab position causes re-binding.

  /* Make the window draggable by glassed toolbars (bug 555081) */
    ...
    #navigator-toolbox[tabsontop="false"] > #nav-bar,
    ... {
>>    -moz-binding: url("chrome://global/content/bindings/toolbar.xml#toolbar-drag");
    }

However, the binding constructor does not call UpdateUrlbarSearchSplitterState.
So this problem happens.
Comment 7 Dão Gottwald [:dao] 2010-11-11 05:16:00 PST
Created attachment 489799 [details] [diff] [review]
patch
Comment 8 Johnathan Nightingale [:johnath] 2010-12-15 09:38:55 PST
No, johnathan, this doesn't block.
Comment 9 Alice0775 White 2011-01-08 03:11:19 PST
*** Bug 624113 has been marked as a duplicate of this bug. ***
Comment 10 Dão Gottwald [:dao] 2011-02-20 14:14:09 PST
Created attachment 513891 [details] [diff] [review]
better patch
Comment 11 Bill Gianopoulos [:WG9s] 2011-02-21 11:12:20 PST
(In reply to comment #10)
> Created attachment 513891 [details] [diff] [review]
> better patch

I find that with this new patch, in order to get the splitter to function, I have to enter/exit "Toolbar Layout" after toggling tabs-on-top.

This is with Windows 7.
Comment 12 Neil Deakin 2011-02-22 06:45:42 PST
Can you explain when someone would use this 'skipintoolbarset' attribute?

Also, tests should be added to test_toolbar.xul
Comment 13 Dão Gottwald [:dao] 2011-02-22 06:56:03 PST
(In reply to comment #12)
> Can you explain when someone would use this 'skipintoolbarset' attribute?

When adding nodes to the toolbar that aren't supposed to participate in toolbar customization, like the splitter. The (val == this.currentSet) check wouldn't work otherwise. this.currentSet would contain urlbar-search-splitter but val wouldn't.
Comment 14 Dão Gottwald [:dao] 2011-02-23 05:40:28 PST
Created attachment 514472 [details] [diff] [review]
test added
Comment 15 tymerkaev 2011-03-16 07:08:33 PDT
*** Bug 642115 has been marked as a duplicate of this bug. ***
Comment 16 mcdavis941 (sporadically reading bugmail) 2011-12-19 14:23:58 PST
Just FYI, I see this almost every day.  When I was looking into this in August (to see if it was something I was causing) I added a little CSS in Stylish to make #urlbar-search-splitter's position obvious:

#urlbar-search-splitter { background-color: lime !important; }

and I never turned that off. So, like I said, with my usage pattern, I still notice this on the order of once a day.  FWIW.
Comment 17 Dão Gottwald [:dao] 2012-03-21 08:22:01 PDT
Created attachment 607966 [details] [diff] [review]
patch

updated to tip
Comment 19 Matt Brubeck (:mbrubeck) 2012-03-26 11:26:26 PDT
https://hg.mozilla.org/mozilla-central/rev/fe40e7c2d790
Comment 20 neil@parkwaycc.co.uk 2012-03-30 13:44:07 PDT
Comment on attachment 607966 [details] [diff] [review]
patch

Bah, I knew I had forgotten somthing...

>+            if (val == this.currentSet)
>+              return;
This should return val.

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