97119 - Inconsistent textbox height in Pref -> Advanced -> Proxies

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect, P4)

Description

[doctor__j]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.3+) Gecko/20010824
BuildID:    2001082406

Reproducible: Always
Steps to Reproduce:
1. Open pref menu.
2. Go to Advanced -> Proxies
3. "Automatic proxy config URL" textbox is thicker than the five textboxes above.

Comment 1

[doctor__j]

Attachment: Screebshot

Comment 2

[doctor__j]

Attachment: Screenshot

Comment 3

[sairuh (rarely reading bugmail)]

benc, the networking ppl own this panel, right?

Comment 4

[Gagan]

No. UI folks do. 

Comment 5

[benc]

I'm here to help, but it sounds like something UI person could drive.

Comment 6

[Gervase Markham [:gerv]]

It's being made too big because it's expanding to fill the extra space created
by the button next door.

Gerv

Comment 7

[Andreas Premstaller]

Attachment: Patch to use consistent line height for textboxes throughout proxy dialog

Comment 8

[Andreas Premstaller]

I have created a patch using Patch-Maker. Added autostretch="never" to
attributes of the hbox to avoid stretching of the text box to the height of the
button in the same hbox.

Tested on Win2000 and Linux using Modern and Classic skin. 

review anyone?

Comment 9

[Andreas Premstaller]

Attachment: Screenshot with patch applied

Comment 10

[Matthew T (active 1999-2002)]

This seems like the wrong fix. You shouldn't have to special-case this text 
field just to make it the usual height; single-line text fields should always 
be the same number of pixels larger than the text inside them, unless 
explicitly set otherwise (which isn't the case here). Also, the post-patch 
screenshot shows that the text field and button aren't lined up correctly; they 
should be vertically centered, not aligned along the top.

Anyway, this isn't a UI Design bug, since the correct design is obvious.
--> Themes

Comment 11

[Andreas Premstaller]

If the default behaviour of XUL-elements is to be changed, this is beyond my
possibilities and probably up to quite some discussion. If it changes, recent
discussions suggest it should happen before Mozilla 1.0.

However, if only the minimal solution to the problem should be followed, I have
improved the patch to align text and button vertically, and have also corrected
alignment in the smart_browsing pref-window.

Attaching patch, screenshots.

hewitt: are you the right person for review or can you direct me to somebody else?

Comment 12

[Andreas Premstaller]

Attachment: Improved patch, adjust text height, alignment in proxy and smart_browsing

Comment 13

[Joe Hewitt (gone)]

valign and autostretch have been deprecated.  Instead, just use <hbox
align="center"/>

Comment 14

[Andreas Premstaller]

Attachment: Pref->Proxy settings after the patch

Comment 15

[Andreas Premstaller]

Attachment: Pref->Smartbrowsing now

Comment 16

[Andreas Premstaller]

Attachment: Patch v0.3 for proxy and smart_browsing-pref

Comment 17

[Andreas Premstaller]

using align="center" exclusively. Also removed the unnecessary occurancies of
autostretch="never" in pref-proxies.xul and pref-smart_browsing.xul.

I don't want to spam bugzilla, and the windows look good -> omitting screenshots.

Comment 18

[Andreas Premstaller]

Attachment: Patch v0.4, missed to some align="center"

Comment 19

[Andreas Premstaller]

hewitt, could you please give a look at the most recent patch?

Comment 20

[Joe Hewitt (gone)]

"right" is a deprecated value for align.  Use align="end" instead

Comment 21

[Andreas Premstaller]

Attachment: Patch v0.5, use align="end" and "start"

Comment 22

[Andreas Premstaller]

hewitt, thanks for your patience. Here is another one (and I found an
align="left"  and replaced it by "start"). No occurrence of autostretch, "right"
or left in the files any more.

Comment 23

[Joe Hewitt (gone)]

this was fixed by blake's recent xul-mega-landing

Comment 24

[Andreas Premstaller]

hewitt: The textbox size is still not ok.

Blake's checkin did not add the necessary align="center" to set the correct
height of the hbox in the proxy window. It fixed autostretch="never" and
"align="center", however, there are still some align="right" in the file. 
And the width of the textboxes is now too small, despite a lot of flex="1".

Please REOPEN.

Comment 25

[Andreas Premstaller]

Attachment: patch v0.6, just correct the textbox height and replace some align="right" by "end"

Patch V0.6 fixes the textbox height and makes some minor syntay corrections.
However, I could need help to stretch the hboxes to full width of the window.
Despite some flex="1" it doesn't seem to work for me.

Comment 26

[Andreas Premstaller]

Ups, I just found out I can reopen. Doing so. Sorry for the spam!

Comment 27

[Joe Hewitt (gone)]

Attachment: patch that really works

That last patch didn't quite do the trick.  align="end" didn't work because, in
an hbox, if you want to right-align your contents you need to use pack="end". 
Also, I noticed the textboxes weren't flexing as they should, and were a little
too short, so I added a align="stretch" to the top radiogroup.

Comment 28

[Andreas Premstaller]

hewitt, thank you for the insight.
I applied your patch, it works perfectly, not really unexpected :-)

Comment 29

[Joe Hewitt (gone)]

fixed

Comment 30

[Patty Mac]

Verified the patch.