Bug 267831 added a splitter to the customize toolbar palette to allow resizing of the search bar, but that ended up: a) not being discoverable to existing users, b) resulting in strangeness when it was moved to be between, say, two buttons Upon reflection and based on feedback in that bug, we should - remove the splitter item from the customize toolbar palette - do something more like attachment 262241 [details] [diff] [review] where we automatically add the splitter between search and location bars allowing them to be resized

I prefer the solution in bug 393733: Allow resizing the search bar in Customize Toolbars mode (without adding a splitter).

Removes the splitter from the customize toolbar window, and just inserts the splitter between the search and urlbar items when they are next to each other. The nsSplitterFrame is actually the fix for bug 391121. It adds support for resizebefore/after="flex" which is equivalent to "closest" except that it only resizes flexible elements.

It would be nice to read some reasoning as to why this solution is better than bug 393733. That is, why add a permanent UI element for something that most users will use never or once? Showing the splitter while customizing (which is the opposite of what your doing) is the way to go, IMHO.

I think the reasoning is to make it more discoverable. In bug 393733, not only does one have to figure out how to customize toolbars, one also has to notice the little dot that appears between the two fields.

(In reply to comment #4) > the little dot that appears between the two fields. Note that the dot is Mac-specific. On Linux and Windows it's a rather ugly vertical bar. In any case, if the splitter would only be visible while customizing, it could be more eye-catching and use a stronger visual metaphor like <http://www.lo-motion.de/t/images/resize.gif>.

Comment on attachment 285466 [details] [diff] [review] use a different mechanism for the search reszier I'm reduced to making really obscure style nits ;-) >- enum ResizeType { Closest, Farthest, Grow }; >+ enum ResizeType { Closest, Farthest, Grow, Flex }; I think this would be neater with Grow last, since only resizeafter uses Grow. > case 0: return Farthest; > case 1: return Grow; >+ case 2: return Flex; (... in which case I would reorder uses such as this into the same order.) > // if the resizebefore is closest we must reverse the list because the first child in the list > // is the Farthest we want the first child to be the closest. >- if (resizeBefore == Closest) >+ if (resizeBefore == Closest || resizeBefore == Flex) > Reverse(mChildInfosBefore, mChildInfosBeforeCount); Since resizeBefore can't be Grow, you could use resizeBefore != Farthest, or if you prefer, reorder Flex before Farthest and use resizeBefore <= Farthest.

Comment on attachment 285466 [details] [diff] [review] use a different mechanism for the search reszier >Index: browser/base/content/browser.js >+ splitter = document.createElement("splitter"); >+ splitter.id = "urlbar-search-splitter"; >+ splitter.setAttribute("resizebefore", "flex"); >+ splitter.setAttribute("resizeafter", "flex"); The splitter should have the "chromeclass-toolbar-additional" class name.

Also, maybe the element should be created lazily to minimize the cost if the location bar and the search bar aren't siblings?

What about a slightly more general implementation: adding a splitter after every flex'ed toolbar item if there's another one on the same toolbar following (resp. adding one before every flex'ed item if we've already passed one while searching)? That would allow to (1) have e.g. Stop and Reload between address and search bar or (2) move the search bar down to the Bookmarks toolbar - while still being able to resize it. Furthermore it's nicer for extension authors (who'd otherwise have to implement the same feature for their own extensions - as do many search toolbar extensions, etc.).

Comment on attachment 286562 [details] [diff] [review] address comments from Neil and Dão >Index: browser/base/content/browser.js >=================================================================== >RCS file: /cvsroot/mozilla/browser/base/content/browser.js,v >retrieving revision 1.870 >diff -u -p -8 -r1.870 browser.js >--- browser/base/content/browser.js 13 Oct 2007 02:43:48 -0000 1.870 >+++ browser/base/content/browser.js 29 Oct 2007 15:47:49 -0000 >@@ -1065,16 +1065,18 @@ function delayedStartup() > Cc["@mozilla.org/login-manager;1"].getService(Ci.nsILoginManager); > > if (gMustLoadSidebar) { > var sidebar = document.getElementById("sidebar"); > var sidebarBox = document.getElementById("sidebar-box"); > sidebar.setAttribute("src", sidebarBox.getAttribute("src")); > } > >+ UpdateUrlbarSearchSplitterState(); >+ > initPlacesDefaultQueries(); > initBookmarksToolbar(); > PlacesUtils.bookmarks.addObserver(gBookmarksObserver, false); > PlacesStarButton.init(); > > // called when we go into full screen, even if it is > // initiated by a web page script > window.addEventListener("fullscreen", onFullScreen, true); >@@ -2151,16 +2153,42 @@ function canonizeUrl(aTriggeringEvent, a > } > > gURLBar.value = getShortcutOrURI(url, aPostDataRef); > > // Also update this so the browser display keeps the new value (bug 310651) > gBrowser.userTypedValue = gURLBar.value; > } > >+function UpdateUrlbarSearchSplitterState() >+{ >+ var splitter = document.getElementById("urlbar-search-splitter"); >+ >+ var urlbar = document.getElementById("urlbar-container"); >+ var searchbar = document.getElementById("search-container"); >+ var ibefore = null; >+ if (urlbar.nextSibling == searchbar) >+ ibefore = searchbar; >+ else if (searchbar.nextSibling == urlbar) >+ ibefore = urlbar; >+ else if (splitter.parentNode) >+ splitter.parentNode.removeChild(splitter); Shouldn't this be |else if (splitter)| >+ >+ if (ibefore) { >+ if (!splitter) { >+ splitter = document.createElement("splitter"); >+ splitter.id = "urlbar-search-splitter"; >+ splitter.setAttribute("resizebefore", "flex"); >+ splitter.setAttribute("resizeafter", "flex"); >+ splitter.setAttribute("class", "chromeclass-toolbar-additional"); nit: use .className. r=mano otherwise.

So is this splitter resize supposed to be the ugly bar again and not the 4 dots?

(In reply to comment #13) > So is this splitter resize supposed to be the ugly bar again and not the 4 > dots? As it is only visible when customizing the toolbar, it should be better visible than the 4 dots which looked nice when they are there always, but lack contrast/discoverability when you open the configure toolbars dialog. PS: I really like the idea of just showing in the customize dialog. Also, why is that customize toolbars dialog so ... ugly? I mean, i like the general idea how it looks, but the icons there are not placed in a grid like e.g. icons in a filemanager, but they just look like they are placed randomly. Also the 1. line has 5 icons, the 2nd and 3rd just 4 icons. Why?

I think it would make much more sense if the splitter would be visible only in customize mode. Currently it is the other way round. And there is an invisible splitter in customize mode now, that can be dragged to a toolbar and causes the splitter to disappear in normal mode.

Could this have caused bug 403724? The tree is currently closed so it would be great if someone could look into it asap.

This bug makes bookmarks toolbar items disappear (when search bar is in customize window).

The bookmarks toolbar items are gone after a restart. There are more issues without search bar on toolbar: after opening customize window the menus turn grey and dysfunctional and you can't open the customize window for a second time. After a restart the menus are functional again until you reopen the customize window. So you can open the customize window only once per session.

Sorry for not being clear, but also the other issues from comment 18 are tested with the hourlies from http://hourly-archive.localgho.st/ and they indicated to this bug as the possible cause but also bug 400872 is still open and maybe this could also have influenced this?

(In reply to comment #18) > you can't open the customize window for a second time. > Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111312 Minefield/3.0b2pre ID:2007111312 I can. no problem with 2nd/3rd.... time.

Backed out to see if that fixes Ts regression in bug 403724

As a side-note, this patch also broke the 'native styling' of the re-sizer. Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b2pre) Gecko/2007111413 Minefield/3.0b2pre Firefox/3.0 ID:2007111413

(In reply to comment #21) > Backed out to see if that fixes Ts regression in bug 403724 Relanded at 2007-11-14 19:38.

The bookmarks problem described in Comment 18 have reappeared in the 2007111505 build, perhaps due to the re-landing. See Bug 403743.

It doesn't seem like there is an about:config pref to easily get rid of that splitter. Adding #urlbar-search-splitter { display:none; } to userChrome.css is rather uncomfortable. Should I open a new bug for this?

(In reply to comment #25) > It doesn't seem like there is an about:config pref to easily get rid of that > splitter.[...] Should I open a new bug for this? see bug 403732 - toolbar resizer cannot be removed

One of the points made above is quite valid, not sure as to why has that scenario not been taken care of. So for example, if a user places a few toolbar buttons between location and search bar, is there going to be no way at all for resizing them?

This regressed the styling from bug 393718 so now we have the ugly thick line again. Someone needs to replace this: + splitter.className = "chromeclass-toolbar-additional"; with this: + splitter.className = "chromeclass-toolbar-additional toolbar-splitter";

(In reply to comment #28) > This regressed the styling from bug 393718 so now we have the ugly thick line > again. See bug 403705.

Mid air collision, I was just going to post that I just noticed that. Sorry :-)

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b2pre) Gecko/2007120704 Minefield/3.0b2pre. Also for linux and windows trunk builds also.

https://litmus.mozilla.org/show_test.cgi?id=5147 in-litmus+