348779 - Searchbar demands too much space

Status: RESOLVED FIXED

(Firefox :: Search, defect)

Description

[Ria Klaassen (not reading all bugmail)]

When you have a lot of other stuff on your toolbar, the searchbar maintains its size at the cost of the locationbar. I think they should at least share the space equally.
This has even become worse when the new searchbar-engine-button was implemented a few days ago.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Attachment: Screenshot of the problem

Comment 2

[Peter Kasting]

Nominating.  Can we tweak the flex values of the elements so that things adjust themselves properly?

Comment 3

[Mike Connor [:mconnor]]

basically there's a min-width on the searchbar, but not on the location bar.  Not going to block on an edge case, but would consider a safe patch.

Comment 4

[Ria Klaassen (not reading all bugmail)]

This bug is also indirectly related to Bug 344941.
Problem is that the flex set in bug 258679 does not work as well as hoped. 
Since Bug 347961 was fixed the width of the box has stayed the same: 15em. As the button was placed outside the box it is a huge box now in total.
When I reduce the width of the searchbar manually with .searchbar-textbox { width: 5em; } (in a userChrome.css or a theme) it cannot maintain that size anymore. When I open the browser it has another size than when I click in the box; then it suddenly resizes. So I should set a max-size and a min-size for the box which will undo bug 258679 but also cause another problem.

  

Comment 5

[Ria Klaassen (not reading all bugmail)]

Attachment: screenshot to demonstrate the changes (flex, width and min-width)

Comment 6

[Ria Klaassen (not reading all bugmail)]

Attachment: screenshot where both the urlbar and the searchbar both have a min-width

Comment 7

[Ria Klaassen (not reading all bugmail)]

I made these changes for the screenshots that demonstrate how it should look like IMHO. The searchbar does not behave unstable here anymore.

browser.xul:

      <toolbaritem id="urlbar-container" align="center" flex="150"
                   
searchbar.css:

.searchbar-textbox {
  width: 10em;
  min-width: 4em;
}

browser.css: 

#urlbar {
  min-width: 7em;
}

Comment 8

[Peter Kasting]

Attachment: branch patch v1

This is slightly different than comment 7 in that it changes the flex to 300 instead of 150.  With our current flex, the urlbar does most of the shrinking until it hits the min width, then the search bar shrinks.  With 150, the bars behave very nicely on shrinking, but the search box takes up a much larger percentage of the toolbar space in general.  300 makes the shrink behavior _mostly_ nice without making the search box _too_ much larger by default, and seemed like a good compromise.

Comment 9

[Peter Kasting]

Comment on attachment 238123 [details] [diff] [review]
branch patch v1

Joe, if you could take a look at the flex change here, you probably have the best idea of how it will be perceived.  The behavior on this patch looks better at small, medium, and large window widths to me, but I haven't spent much time in here, comparatively.

Comment 10

[Peter Kasting]

Attachment: branch patch v2

After much discussion with Joe, testing on multiple OSes, and reading some XUL documentation (dead tree form), this updated patch should work much better.

The key here is that we don't want the toolbar to start getting into "negative flex" mode, where the available space is less than the desired space.  So, we reduce the widths of the flexing boxes to match the min-widths so that everything is always scaled up beyond the width, instead of down below it.  This maintains the proportionality of the address bar to the search bar.

Then we can set the address bar to flex proportionally; on my system, it turns out that the closest match to current branch/trunk looks to be when the url bar's flex ~= 400.

Finally, Joe's crazy urlbar deck hack is no longer needed, and screwed up all these flex calculations, so it is removed.  Much of the size of the patch comes from whitespace changes around this.

Overall I consider this behavior much more sane, and I understand what's going on a lot better than in my first patch :).

Comment 11

[Peter Kasting]

Attachment: diff -w version of branch patch v2

For convenience in reviewing, here's a diff -w version of the last patch.

Comment 12

[Joe Hughes]

Comment on attachment 238271 [details] [diff] [review]
branch patch v2

Nice.  This doesn't seem to manifest the problem that prompted my horrible stack hack in bug 337427--perhaps other theme-related restructuring fixed the underlying problem in the interim.  The searchbar still flexes, and tends to be larger than it was in FF1.5, which is also good.  (Tested on Mac and Linux.)

Comment 13

[Mike Connor [:mconnor]]

Comment on attachment 238271 [details] [diff] [review]
branch patch v2

r+a=me.  Yay to killing stupid hacks.

Comment 14

[Peter Kasting]

Fix checked in on branch by timeless:

mozilla/browser/themes/pinstripe/browser/browser.css 1.11.4.47
mozilla/browser/themes/pinstripe/browser/searchbar.css 1.4.8.12
mozilla/browser/themes/winstripe/browser/browser.css 1.17.2.59
mozilla/browser/themes/winstripe/browser/searchbar.css 1.3.8.20
mozilla/browser/base/content/browser.xul 1.268.2.66

Not yet fixed on trunk.  Leaving open until I can get this in on trunk, presumably tomorrow.

Comment 15

[Peter Kasting]

Checking this in as-is on current trunk isn't a good idea; because the searchbar structure is different, this makes the searchbar VERY narrow.  I could change the values in my patch to give the search bar a larger width + min-width, which would change this behavior, but it seems better to just wait for the new theme to land and then fix.

Comment 16

[s.spiess]

Firefox 2.0 RC3: http://liberation.ch.vu.seth.hostorama.ch/bilder/FF20RC3.JPG
Firefox 1.5.0.7: http://liberation.ch.vu.seth.hostorama.ch/bilder/FF1507.JPG

Comment 17

[Peter Kasting]

(In reply to comment #16)
> Firefox 2.0 RC3: http://liberation.ch.vu.seth.hostorama.ch/bilder/FF20RC3.JPG
> Firefox 1.5.0.7: http://liberation.ch.vu.seth.hostorama.ch/bilder/FF1507.JPG

Yes, the search bar has flex in 2.0 so it can auto-expand.  This was a requested feature and is expected behavior, not a bug.

Comment 18

[Peter Kasting]

Attachment: Trunk patch (last remaining bit)

Most of the branch patch got landed on the trunk during the theme landing.  All that's left in this patch is removing one unused bit of CSS; no visible effect, just code cleanup.

Comment 19

[Peter Kasting]

Last remaining cleanup bit fixed on trunk:
mozilla/browser/themes/winstripe/browser/browser.css 1.57