181453 - url box too close to personal toolbar in modern theme

Status: VERIFIED FIXED

(SeaMonkey :: Themes, defect)

Description

[Petrus Lundqvist]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826

Ever since the toolbar collapsing buttons were removed from the left side of the
toolbars, the url box has been too low and too close to the personal toolbar in
the modern theme.

Reproducible: Always

Steps to Reproduce:
1. Make sure the modern theme is used
2. Open a browser window
3. Look at the URL input field and the box around it and notice its vertical
alignment
4. Notice how the url box and input field are not vertically centered in the
navigation toolbar but rather they are aligned to the bottom of the toolbar and
much too close to the personal toolbar below it

Comment 1

[Felix Miata]

While the URL bar is no longer vertically centered, it is not yet close enough
to the personal toolbar, and the space above it needs to be reduced to match.
See bug 135457 and the bugs it depends on, all of which are liable to be moot
when fix for bug 15144 lands.

Comment 2

[Petrus Lundqvist]

Bug 135457 represents your personal subjective opinion about the toolbars in
general whereas this bug is reporting an unvoluntary change in the looks of the
toolbar in one specific theme (Modern). This is about restoring visual esthetics
and correctness and can be fixed regardless of 135457. 

Bug 15144 may not be fixed for quite some time since it's pretty major and bug
135457 may never be fixed. This bug however, is a very simple matter of
vertically centering a user interface control.

Comment 3

[Scott McPeak]

Attachment: screenshot of old (good) vs. new (bad) behavior

I'm seeing this on Linux.  It happens for both 1.2.1 and 1.3b.	I don't
have other versions to compare with (and for some reason mozilla's ftp
server isn't responding right now so I can't download another).

Comment 4

[Michael O.]

I don't know how to submit a patch for this, but I do know how to fix it.

In skin/modern/navigator/navigator.css (modern.jar), line 382, attribute for
#nav-bar-inner:

  margin: 4px 0px 3px 5px;

The existing values are |7px 0px 0px 5px|, so there is no bottom margin and the
top margin is a bit big. The new values result in an aesthetically pleasing display.

Comment 5

[Hanspeter Niederstrasser]

Attachment: Vertically centers the urlbar on the Nav Toolbar

Vertically aligns the urlbar so that it is centered on the Navigation Toolbar.

Comment 6

[Scott McPeak]

Attachment: Another set of screenshots, including proposed patch

The attached screenshot shows the original (mozilla 1.0) appearance,
the current (mozilla 1.3) appearance, and that of the 'margin' patch.

The patch restores the spacing between the URL bar and the edge of the
panel, but does not fix the corresponding spacing w.r.t. the back
button nor the throbber.  In particular, the tops of these items no
longer line up with the URL bar.

I suspect the problem arose not from someone tweaking navigator.css, but
from a bug introduced or fixed in the code that interprets it.	But if it
can be fixed from within navigator.css then all's well that ends well.

Comment 7

[[e-mail]]

Attachment: My fix because I broke it :-)

I removed the 45px minimum height on primary toolbars, which allowed the
toolbar to shrink to 40px to "fit" the urlbar; this patch adds some margin
which should restore the previous height.

Comment 8

[Jan Varga [:janv]]

Comment on attachment 124815 [details] [diff] [review]
My fix because I broke it :-)

looks good, tested on linux

Comment 9

[Scott McPeak]

Attachment: Fix the reproduces mozilla-1.0 pixel measurements

This is an improvement on attachment 124815 [details] [diff] [review] that I believe more
accurately reproduces the mozilla-1.0 behavior.  In the next
comment I'll explain more.

Comment 10

[Scott McPeak]

Attachment 124886 [details] [diff] is the result of tweaking navigator.css to get the
pixel distances between the various UI components to match those of
mozilla-1.0.  I think this is important because the previously proposed
patches still have some elements in different positions than what the
theme designer intended.  I also happen to think they look better in
their original positions.

To validate this, I measured the distances between UI elements, counting
only the solid gray pixels that come from the panel background pixmap.
You can compare these counts to the images in Attachment 124736 [details].

Below are those measurements.  I apologize for the length of this comment, 
but I wanted to move away from the imprecise "looks ok to me" approach,
since it's subjective.  This is what I found:

Version A: mozilla-1.0:
summary: this is the original layout of the nav bar in the modern
theme, as far as I'm aware, and the goal is to get mozilla to once
again have this layout

  back button:
    top: 6
    bottom: 6   (does not include nearly-gray from pixmap)

  down-arrow near back button:
    top: 29
    bottom: 3

  throbber:
    top: 6
    bottom: 3

  menu/nav divider
    6         (pixels between elements)
  urlbox top edge
    3
  urlbar text widget
    3
  urlbox bottom edge
    3
  nav/personal divider


Version B: mozilla-1.3:
summary: like Version A, but the entire nav bar is 3px shorter, with
slack entirely taken from the bottom margins except for the throbber,
which took 1px from top and 2px from bottom

  back button:
    top: 6
    bottom: 3

  down-arrow:
    top: 29
    bottom: 0

  throbber:
    top: 5
    bottom: 1

  menu/nav divider
    6
  urlbox top edge
    3
  urlbar text widget
    3
  urlbox bottom edge
    0
  nav/personal divider


Version C: mozilla-1.3, nav-bar-inner margin "4px 0px 3px 5px"
(this is Attachment 124118 [details] [diff])
summary: like Version B, but urlbox is moved 3px higher

  back button:
    top: 6
    bottom: 3

  down-arrow:
    top: 29
    bottom: 0

  throbber:
    top: 5
    bottom: 1

  menu/nav divider
    3
  urlbox top edge
    3
  urlbar text widget
    3
  urlbox bottom edge
    3
  nav/personal divider


Version D: mozilla-1.3, nav-bar-inner margin "7px 0px 5px 5px"
(this is Attachment 124815 [details] [diff])
summary: like Version A, but:
  - down-arrow is shifted down 2px
  - throbber is shifted down 1px
  - urlbar text widget's margin top & bottom margins reduced by 1px,
    slack given to the urlbox bottom margin

  back button:
    top: 6
    bottom: 6
    
  down-arrow:
    top: 31
    bottom: 1
    
  throbber:
    top: 7
    bottom: 2
    
  menu/nav divider
    6
  urlbox top edge
    2
  urlbar text widget
    2
  urlbox bottom edge
    5
  nav/personal divider


Version E: mozilla-1.3, with:
  #nav-bar-buttons margin "0px 3px 3px 0px"
  #nav-bar-inner margin "7px 0px 3px 5px"
  #throbber-box margin "0px 0px 2px 0px"
(this is my Attachment 124886 [details] [diff])  
measurements are identical to version A!

Comment 11

[Hanspeter Niederstrasser]

From Attachement 124886:

> #nav-bar-buttons {
>-  margin-left: 3px;
>+  margin: 0px 3px 3px 0px;

Shouldn't this be
+  margin: 0px 0px 3px 3px;
That is, the margin-left is mantained, rather than shifted to margin-right?

ps.  OS/hardware should be all/all

Comment 12

[Scott McPeak]

Attachment: Yet another improved patch

Yes, tryandguessit is right.  Included is a fixed patch.  (For some
reason bugzilla isn't listing the old patch as being among those I
can make obsolete, oh well...)

Comment 13

[jag (Peter Annema)]

Actually, do we really need this change:

 #nav-bar-buttons {
-  margin-left: 3px;
+  margin: 0px 0px 3px 3px;
 }

That part makes the toolbar three pixels taller. Is there another way to center
the buttons / throbber better?

Comment 14

[jag (Peter Annema)]

It looks like we could take a pixel or two from the top.

Comment 15

[jag (Peter Annema)]

Comment on attachment 124969 [details] [diff] [review]
Yet another improved patch

Cancelling sr= for now

Comment 16

[Scott McPeak]

Attachment: screenshot of reverting nav-bar-buttons change

According to my measurements, and as shown in the attached screenshot,
if I remove the change to "nav-bar-inner" then we have:

Version F: same as E but removing the nav-bar-buttons change:
summary: nav bar is two pixels shorter than in Version A, with
the down-arrow much too close (0px) to the bottom edge

  back button:
    top: 6
    bottom: 4
    
  down-arrow:
    top: 30
    bottom: 0
    
  throbber:
    top: 5
    bottom: 2

  nav/menu divider
    6
  urlbox top edge
    2
  urlbar text widget
    2
  urlbox bottom edge
    3
  nav/personal divider

My goal was to get the navbar to have the same inter-element spacings
as in mozilla-1.0, since I prefer it that way and that was how the
theme designer had it.

But even if we want it 2px shorter than in 1.0, the back button's little 
down-arrow should (IMO) not be touching the bottom edge.

Comment 17

[Scott McPeak]

>if I remove the change to "nav-bar-inner" then we have:

I meant "nav-bar-buttons" here.

Comment 18

[jag (Peter Annema)]

Comment on attachment 124969 [details] [diff] [review]
Yet another improved patch

Scott: understood. I was wondering if we made the toolbar a little less high on
purpose (to save some vertical space, every pixel counts), and if that's the
case, could we leave it at the current height, and just move all the buttons up
a little so the little arrow on back/forward/print doesn't touch the bottom of
the toolbar?

However, since you have r=shliang, sr=jag.

Comment 19

[Scott McPeak]

This bug is tricky because it involves people's subjective judgment
about aesthetics; one person's pleasing space is another's wasted
pixels.  Obviously, alternative themes are inteded to address the
demand for tighter UI layouts, but alternative themes usually don't
get the same QA that the default shipping themes do.  So I understand
the tendency to opportunistically improve the default themes.

But I'd point out that if I were the original theme designer, having
spent time getting it to look the way I want and attracting a
following of happy users in the process, I'd be skeptical about some
engineers "fixing" my theme just because they were already changing
related code to solve another problem.  We (engineers) often have a
poor sense of how a UI will be perceived by users (case in point: I
failed to notice the reversed margin around nav-bar-buttons, due to
focusing on vertical spacing).

Therefore, in the absence of additional input from the theme designer,
I think the first priority is to do our best to restore the original
design.  It shows respect for the design and designer, and thereby
encourages non-engineers to continue to contribute.  We can then
consider preferences like those expressed in Bug 135457 as a separate
issue, perhaps soliciting input from theme designers for that process.
But in that vein, consider that Bug 135457 has 0 votes, while 
Bug 15144 (which addresses horizontal spacing) has 59.

Comment 20

[Samir Gehani]

adt: nsbeta1-

Comment 21

[jag (Peter Annema)]

Checked in.

Comment 22

[Mike Kowalski]

Comment on attachment 124969 [details] [diff] [review]
Yet another improved patch

polish up the browser. I don't think it's a high risk patch.

Comment 23

[Grace Bush]

verified Mozilla and commercial 6/11 builds

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 124969 [details] [diff] [review]
Yet another improved patch

moving approval request forward.

Comment 25

[Andreas Kunz]

*** Bug 175170 has been marked as a duplicate of this bug. ***

Comment 26

[Mike Kaply [:mkaply]]

Comment on attachment 124969 [details] [diff] [review]
Yet another improved patch

a=mkaply

Comment 27

[Mike Kaply [:mkaply]]

Please add the fixed1.4.1 keyword when this is checked in.

Comment 28

[Mike Kowalski]

fixed on 1.4.x branch.