263750 - -moz-appearance: none gives the urlbar a 2px inset border regardless of gtk theme

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect)

Description

[logan]

The changes from bug 248419 make the urlbar look more like a text input box,
adding an extra 1px black border on the top and left. There was much talk of
-moz-appearance sucking, not sure if this was expected.

It's my understanding that the yellow urlbar in windows has no such border, if
this is wrong and that was intended, please forgive.

I've used the following to improve the appearance, but I'm sure this is blah...

  #urlbar[level="high"] > .autocomplete-textbox-container,
  #urlbar[level="low"] > .autocomplete-textbox-container,
  #urlbar[level="broken"] > .autocomplete-textbox-container {
    border: 1px solid transparent !important;
    padding: 1px !important;
  }

I'll attach a screenshot of the before and after.

Comment 1

[logan]

Attachment: screenshot

Comment 2

[Tuukka Tolvanen (sp3000)]

Attachment: screenshot, industrial gtk theme

given a flat and rounded enough theme, the uglification can be more serious

Comment 3

[David P James]

I don't think this is Linux-only since according to a Windows-based screenshot 
in bug 263754 I see the black inset too 
(https://bugzilla.mozilla.org/attachment.cgi?id=161698&action=view) 
 
OS --> All 

Comment 4

[Tuukka Tolvanen (sp3000)]

David, in attachment 161698 [details], the border looks the same for http and https in
classic Windows style, which is as it should be, and bug 263754 covers the width
difference issue on Windows. According to bug 248419 comment 13, bug 248419 did
not break luna theming of the location bar border.

Comment 5

[Steven Garrity [:sgarrity]]

Attachment: Screenshot of the bug with Bluecurve GTK theme

Comment 6

[Kevin Gerich]

Because the urlbar is drawn-

Comment 7

[Kevin Gerich]

Argh, sorry for the spam. Because the Gnomestripe urlbar is styled in two parts
by -moz-appearance (the autocomplete textbox container and the history
dropmarker), we can't give the textbox container a background color (to indicate
a secure site) and a -moz-appearance at the same time. -moz-appearance overrides
backgrounds and borders.

Windows doesn't have this problem because the urlbar is styled, not the inner
boxes. We could give the urlbar a -moz-appearance: textfield, but then the
dropmarker would appear to be inside the urlbar box which would look odd on Linux.

Not sure what to do here. Suggestions?

Comment 8

[Kevin Gerich]

Attachment: Add a box to the urlbar and give it that mustardy color when on a secure site

OK, here's my solution. I add a box to the inside of the
autocomplete-textbox-container and then give it the secure site color instead
of the container itself. So we don't need the -moz-appearance: none.

I don't have a Linux box to test with. Could someone help me out and please
test the patch?

Comment 9

[logan]

Attachment: screenshot

Looks better, but there's a kindof white box around the yellow urlbar.

Comment 10

[logan]

Attachment: closeup

here's a closeup with a png, easier to see it..

if it's a choice between the two, I'd take this one, less weirdness to disable
:)

Comment 11

[logan]

Attachment: grr

ya, sorry.. that wasn't a closeup at all heh :S

Comment 12

[Kevin Gerich]

That white box might be the border of the autocomplete-textbox-container. Can
you try other themes and see if it changes?

Comment 13

[logan]

CleanIce and ThinIce both show the 1px white border. The default Gtk theme has a
1px black border, like the first screenshot, but this time it's also on the
search thingy.

Comment 14

[Steven Garrity [:sgarrity]]

I've tried this latest patch out with a few themes (Bluecurve, Industrial,
Simple), and they all look fine. They do that the 1-px white inside border, but
I can live with that. It's a significant improvement over the previous issue.

Comment 15

[Kevin Gerich]

Steve helped me investigate this and the inner "white border" is the actual
border which is being drawn by -moz-appearance. You'll see the same thing in the
Find Bar when your search term is not found and the textfield background turns red.

Check out http://actsofvolition.com/images/screenshots/gnome-firefox-largefonts2.png

We can remove the inner pixel border with a negative margin, but that will make
other themes, and probably Luna, look awkward.

Comment 16

[Ben Goodger (use ben at mozilla dot org for email)]

Can't you use a gnomestripe specific stylesheet? 

Comment 17

[Kevin Gerich]

I suppose we could put this in gnomestripe/global/autocomplete.css. Gnomestripe
doesn't have its own browser.css file. Do we want to create one just for this
issue?  

Comment 18

[Kevin Gerich]

Attachment: Apply binding and style in gnomestripe/global/autocomplete.css

Is this what you had in mind Ben? Can someone please test this for me quickly?
Thanks.

Comment 19

[Tuukka Tolvanen (sp3000)]

http and https urlbars look fine over here with attachment 164481 [details] [diff] [review], fx gtk2
aviary, industrial. (https with the 1px white around the yellow, as expected)

Comment 20

[Kevin Gerich]

Comment on attachment 164481 [details] [diff] [review]
Apply binding and style in gnomestripe/global/autocomplete.css

Thanks for testing. Requesting approval to land..

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 164028 [details] [diff] [review]
Add a box to the urlbar and give it that mustardy color when on a secure site

too late for 1.0 (and not terribly ugly.)

Comment 22

[Asa Dotzler [:asa]]

Comment on attachment 164481 [details] [diff] [review]
Apply binding and style in gnomestripe/global/autocomplete.css

too late for 1.0 (and not terribly ugly.)

Comment 23

[Steven Garrity [:sgarrity]]

This was fixed pre-1.0, but it was too close to the 1.0 launch to land - despite
my best back-room dimplomacy efforts ;-)

Can this be landed now?

Comment 24

[Mike Connor [:mconnor]]

Comment on attachment 164481 [details] [diff] [review]
Apply binding and style in gnomestripe/global/autocomplete.css

>Index: browser/themes/winstripe/browser/browser.xml
>===================================================================
>RCS file: /cvsroot/mozilla/browser/themes/winstripe/browser/browser.xml,v
>retrieving revision 1.1.2.1
>diff -u -r1.1.2.1 browser.xml
>--- browser/themes/winstripe/browser/browser.xml	5 Jun 2004 21:31:21 -0000	1.1.2.1
>+++ browser/themes/winstripe/browser/browser.xml	3 Nov 2004 23:34:01 -0000
>@@ -23,4 +23,13 @@
>     </content>
>   </binding>
> 
>+  <binding id="autocomplete-security-wrapper" extends="xul:hbox">
>+    <content>
>+      <xul:hbox id="autocomplete-security-wrapper" flex="1">
>+        <children/>
>+      </xul:hbox>
>+    </content>
>+  </binding>
>+
>+
> </bindings>

just one line here. 


r=me, but I wonder if this is "good enough" or if we have more room for
improvement.

Comment 25

[Steffen Wilberg]

Checking in mozilla/browser/themes/winstripe/browser/browser.xml;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.xml,v  <--  browser.xml
new revision: 1.3; previous revision: 1.2
done
Checking in mozilla/browser/themes/winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.12; previous revision: 1.11
done
Checking in mozilla/toolkit/themes/gnomestripe/global/autocomplete.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/global/autocomplete.css,v  <-- 
autocomplete.css
new revision: 1.6; previous revision: 1.5
done

Comment 26

[Kevin Gerich]

Thanks Steffen!

Comment 27

[Takanori MATSUURA]

Attachment: URLbar of firefox trunk on Fedora Core 3

A check-in of toolkit/themes/gnomestripe/global/autocomplete.css is
right?	On my Fedora Core 3 box, URL bar is broken on my home made
firefox-trunk with gnomestripe.

This problem is fixed when I revert the checkin of
themes/gnomestripe/global/autocomplete.css to previous version.

Comment 28

[Steffen Wilberg]

This is probably an issue with the theme you're using. Try the default theme.

Comment 29

[Kevin Gerich]

My trunk build on FC3 works as expected with the default theme. I don't see how
this patch would cause breakage in addon themes..

Thanks for checking this in Steffen! 

Comment 30

[:Gavin Sharp [email: [e-mail]]]

This patch should not have added browser-specific styles to toolkit's autocomplete.css. I filed bug 314663 for that issue.