893011 - Border in find bar on top wrongly placed after bug 869543

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: findbarOnTopBorder.jpg

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0 (Beta/Release)
Build ID: 20130618035212

Steps to reproduce:

As I mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=869543#c14, an adjustment is needed on windows, background-image property of find bar adds a pseudo-border on top.

The upper part of the screenshot is how it looks like now, in the bottom part I added the following custom declarations to the findbar as a test, personally I think it looks good:

background-position: bottom;
background-image: linear-gradient(rgba(255,255,255,0.15) 1px, rgba(0,0,0,0.15) 1px) !important;

The background-image property is the inverse of what it looks like at the moment.

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Patch: fix findbar border on Windows

Comment 2

[Luís Miguel [:quicksaver] (not reading bugmail)]

Looking at that code, I'm guessing the box-shadow property was added to the find bar on top to provide the same effect of a pseudo-border. I assume this because that property was also added with bug 869543.

I think that property should be removed as it essentially places the shadow directly behind the linear-gradient used in background-image, making it pretty much useless or, even worse, accentuates more the pseudo-border by adding its shading to the one from background-image.

This is a redundant effect in my opinion. I suggested the background-position and background-image combination because it provides the same effect as before, thus being in conformity with the other OS's which also keep the same effect.

Comment 3

[Luís Miguel [:quicksaver] (not reading bugmail)]

By the way, when I say box-shadow property, I'm referring to this one:

box-shadow: 0 -1px 1px rgba(0,0,0,.1) inset;

in line 20 of a/toolkit/themes/windows/global/findBar.css

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 774705 [details] [diff] [review]
Patch: fix findbar border on Windows

Mirroring the image vertically seems like it would give us a broken 3D effect with a highlight at the bottom of the bar rather than at the top.

Comment 5

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: Findbar with border fixes

Personally I disagree, I think it just gives us a normal-looking border at the bottom, with a small contrasting 1px line. Shouldn't it contrast with the content which could be dark, rather than contrasting with the navigation bar which will always be that color with a border visible? This is the exact same effect it had with the find bar on the bottom, as well as the same effect the linux css file has for the current find bar on top.

If you're speaking about my screenshot, that may be true but it's because of compression, it seems like there's a blue shadow below the border. I've uploaded a new screenshot with the fixes proposed in Mike's patch + removed the box-shadow line I mentioned in comment 2.

Comment 6

[Mike de Boer [:mikedeboer]]

Attachment: Patch v2: fix findbar border on Windows

removed box-shadow property. Thanks for being on top of this Quicksaver!

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 777064 [details] [diff] [review]
Patch v2: fix findbar border on Windows

I still don't think we want the background image mirrored to the bottom. Usually a top highlight conveys an outset appearance whereas a bottom highlight conveys an inset appearance, neither of which I think we want here.

Comment 9

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: Findbar with single line border proposal

Of course, I agree with you in theory, although no matter how I look at the findbar I just don't see that 3D effect, maybe because it's too subtle (or maybe because I just have a too positive look on things I guess).

You could lose the white pixel line and leave only the dark one if you feel so strongly about it. The difference is not that much I think.

Screenshot: Another alternative to keep the 3D effect you mention would be to place just the white line at the top (no black line) and use a box-shadow to place the dark line at the bottom to work as a border (it's not the original box-shadow, I tweaked it a bit so it doesn't look so blurry):
findbar {
    background-image: linear-gradient(rgba(255,255,255,0.15) 1px, rgba(0,0,0,0) 1px);
    box-shadow: 0px -1px 0 rgba(0,0,0,0.18) inset;
}

I still prefer the current patch's look honestly, but like I said, maybe that's just me.

Comment 10

[Luís Miguel [:quicksaver] (not reading bugmail)]

Or, of course, just the single line box-shadow for the border and no background-image to make it look flat.

Comment 11

[Mike de Boer [:mikedeboer]]

Attachment: Patch v3: fix findbar border on Windows

Comment 12

[Mike de Boer [:mikedeboer]]

Attachment: Patch v3: fix findbar border on Windows

Carrying over r=dao.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e1bf70be67ec

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e1bf70be67ec