Closed Bug 351195 Opened 18 years ago Closed 18 years ago

Address bar should be non-native on Linux

Categories

(Firefox :: Toolbars and Customization, defect)

2.0 Branch
x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 2

People

(Reporter: pkasting, Assigned: myk)

References

Details

(Keywords: fixed1.8.1, Whiteboard: [Fx2 theme change])

Attachments

(6 files, 2 obsolete files)

On Linux, the search bar is already (apparently) non-native and looks fine with the endcap buttons. However, the address bar appears to be natively themed, which looks very bizarre, since it's typically taller than the go button endcap (and the endcap doesn't currently scale), and often has rounded ends that look very odd with the endcap or in comparison to the search bar. See the right column of https://bugzilla.mozilla.org/attachment.cgi?id=236546 , which shows both bars with a variety of different Linux themes. I think we should bite the bullet and make the address bar non-native on Linux like the search bar appears to already be. Note that bug 347616 would make the dropmarker non-native.
Flags: blocking-firefox2?
Target Milestone: --- → Firefox 2
The second post on this page: http://forums.mozillazine.org/viewtopic.php?t=456747&start=45 Proposes some CSS changes (with before/after pix) that might clean up the look of things a bit on Windows Classic. Not sure whether I want to morph this bug into that, but certainly the "address bar is too tall for Go button" gets fixed by those changes.
(In reply to comment #1) > The second post on this page: > > http://forums.mozillazine.org/viewtopic.php?t=456747&start=45 > > Proposes some CSS changes (with before/after pix) that might clean up the look > of things a bit on Windows Classic. Not sure whether I want to morph this bug > into that, but certainly the "address bar is too tall for Go button" gets fixed > by those changes. See also bug 351157 comment 16. This border style looks a lot better...
taking to investigate for now
Assignee: nobody → mconnor
Flags: blocking-firefox2? → blocking-firefox2+
That border style really didn't play so well with classic, especially darker color schemes, I can post some screenshots. I'd love to see this on Linux with Pam's patch.
(In reply to comment #4) > That border style really didn't play so well with classic, especially darker > color schemes, I can post some screenshots. I believe the screenshots here: https://bugzilla.mozilla.org/attachment.cgi?id=236954 ...are styled using ThreeDShadow instead of ThreeDDarkShadow, but I'm not sure. That seems better than what we have now, at least.
(In reply to comment #5) > I believe the screenshots here: > https://bugzilla.mozilla.org/attachment.cgi?id=236954 ...are styled using > ThreeDShadow instead of ThreeDDarkShadow, but I'm not sure. Yes, they are. Attachment 236722 [details] also uses ThreeDShadow.
Attached patch patch v1: removes native styling (obsolete) (deleted) — Splinter Review
Here's a patch that removes native styling on Linux for autocomplete widgets. It accomplishes this by simply removing autocomplete.css from gnomestripe, which then falls back on the autocomplete.css in winstripe. autocomplete.css is almost exactly the same in gnomestripe as it is in winstripe once you remove native styling. The only remaining difference between the two files is the fix for bug 270993, which this patch regresses. But it's unclear why we ever fixed that bug in the first place, and if we do want that fix, then I suspect we're better off doing %ifdef MOZ_WIDGET_GTK2 in winstripe's version of the file than maintaining two copies of the file for it. Besides fixing this bug, this patch also basically fixes the portion of bug 351618 which is a followup to bug 347616, since bug 347616's changes to winstripe's autocomplete.css get applied to Linux with this patch applied. I should note, however, that although bug 347616's new autocomplete widget looks good on Linux at the default font size with this patch applied, it contains an artifact at larger sizes, so there may still be a bug in bug 347616's implementation on Linux.
Assignee: mconnor → myk
Status: NEW → ASSIGNED
Attachment #237218 - Flags: review?(mconnor)
https://bugzilla.mozilla.org/attachment.cgi?id=237236&action=view shows a screenshot of this patch on Linux, right? So how come the right edge of the bar is still rounded (and thus doesn't match up with the Go endcap)? Is there more needed here in order to make the box corner(s) square?
(In reply to comment #8) > https://bugzilla.mozilla.org/attachment.cgi?id=237236&action=view shows a > screenshot of this patch on Linux, right? So how come the right edge of the > bar is still rounded (and thus doesn't match up with the Go endcap)? Is there > more needed here in order to make the box corner(s) square? I tested out the combination of this patch and the patch on bug 348138, and one of them (I think this one) has caused some goofiness. My search box used to look good -- the endcaps linked up to a square-ended input box. Now my search box is rounded just like my URL bar, so the endcaps no longer join properly. When I tried other themes, I found my search and URL bars got border effects from the themes, like Classic-esque "sunken" bevels and other things; also, the box size didn't always match the size the endcaps scaled to-- it was frequently off by a pixel on the top and bottom. So I don't think this patch does what I wanted. HOWEVER, dropping the second post on this page: http://forums.mozillazine.org/viewtopic.php?t=456747&start=45 into my userChrome.css atop these patches DID work. I imagine it probably would also work without having applied this patch.
That patch affects all autocomplete text boxes, right (including the search bar)? I'm not sure making such a change for all users of the toolkit is a good idea, especially when we're doing it just to make the Firefox location bar look better. How does this affect the autocomplete text box in the File->Open Location dialog, for example (you need to hide the Navigation toolbar bar to see it)? I was thinking that http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/themes/winstripe/browser/browser.css&rev=1.44#974 was the code that needed to be tweaked for this, since it only applies to the location bar, and comment 0 says that the search bar is already "OK", but I guess you'd then need to be careful to not regress bug 263750.
(In reply to comment #10) > That patch affects all autocomplete text boxes, right (including the search > bar)? Yes, I believe it does. > I'm not sure making such a change for all users of the toolkit is a good > idea, especially when we're doing it just to make the Firefox location bar > look better. An autocomplete widget looks roughly like this (with everything inside the textbox being anonymous content): <textbox type="autocomplete" ... /> <hbox class=".autocomplete-textbox-container" ... /> <dropmarker ... /> <popupset ... /> </textbox> There are four differences in autocomplete.css between winstripe and gnomestripe: 1. winstripe leaves the default style for the textbox and hbox: outer textbox: -moz-appearance: textfield inner hbox: -moz-appearance: none while gnomestripe overrides it with custom style: outer textbox: -moz-appearance: none inner hbox: -moz-appearance: menulist-textfield 2. winstripe makes .autocomplete-textbox-container be -moz-box-align: center, while gnomestripe does not; 3. winstripe contains Pam's dropmarker fixes from bug 347616, while gnomestripe does not; 4. winstripe makes the .autocomplete-tree tree anonymous element (not shown above) be background-color: transparent, while gnomestripe makes it be background-color: -moz-field. Taking these in reverse order: #4 is a questionable fix for bug 270993. It's unclear why this change was made, as the bug never justifies it. I think we're ok reverting it, although we could also leave it in via an %ifdef MOZ_WIDGET_GTK section to winstripe's autocomplete.css. #3 is the dropmarker fix from bug 347616. We want this for gnomestripe. Pam filed followup bug 351618 on it. #2 was checked in to winstripe with the comment "Landing the Aviary Branch (Toolkit sections excluding toolkit/content)." Presumably it was part of a much larger checkin; perhaps it should have landed in gnomestripe, too. In any case, it seems harmless to apply to gnomestripe, and it doesn't cause any problems in my tests. #1 is a small but significant difference between the two themes. It's unclear why gnomestripe overrides styles this way (the checkin comment from 2004-06-18 says merely "More Winstripe/Pinstripe Restructuring"), but it makes applying Pam's dropmarker fixes harder, since those fixes assume that the dropmarker is inside the box with borders, but these changes make the dropmarker be next to the box with borders. We basically have two options here to get both Pam's dropmarker changes and the styling this bug is about: 1. Make gnomestripe use winstripe's autocomplete.css and then apply the style fixes from the mozillazine forum to the location and search bars. 2. Apply Pam's changes to gnomestripe's autocomplete.css (but perhaps with some changes), apply the style fixes from the mozillazine forum to the location and search bars for Windows only, and then apply the same style fixes to different elements for Linux only. To me it sounds like #1 is the safer, simpler option. > How does this affect the autocomplete text box in the File->Open > Location dialog, for example (you need to hide the Navigation toolbar bar to > see it)? It makes it look pretty.
> So I don't think this patch does what I wanted. Right. It was quite incomplete. Here's a patch that does what I think you want. Note that I haven't tested it on Windows yet (I don't have access to a Windows machine today), and I also haven't tested it with RTL; so it may still need a few tweaks. What it does is implement option #1 from comment 11, i.e.: 1. as with the previous patch, removes gnomestripe's autocomplete.css so that gnomestripe uses winstripe's version of that file; 2. applies the changes suggested in the mozillazine forum to the location bar and the search bar, except that it uses the color ThreeDShadow instead of ThreeDDarkShadow as the border color. For some reason I had to move dist/ out of the way when rebuilding to get my build to apply these changes.
Attachment #237218 - Attachment is obsolete: true
Attachment #237361 - Flags: review?(mconnor)
Attachment #237218 - Flags: review?(mconnor)
Bug 263750 comment 7 makes me worry about the Mozillazine forum fixes. Does this fix still work properly on secure sites? Also, when I tested the ThreeDShadow stuff locally, it looked odd, because the endcap images have a particular color/opacity, and "ThreeDShadow" didn't match it; so where the endcap images met the text boxes, things looked wrong. Instead of using ThreeDShadow we should presumably be using the same color/opacity as the endcap images. Finally, I'm concerned about changing the way autocomplete widgets look in other places when our goal was to fix theme in these two specific spots. I'm still not sure why the short-term fix can't be for Gnomestripe's location bar to get styled like its search box is. I don't think that's quite as all-inclusive as your "fix #2" above, but maybe I'm wrong.
Here's a screenshot showing what the location bar, search bar, and "Open Web Location" dialog look like before and after the patch.
Attachment #237364 - Attachment is patch: false
Attachment #237364 - Attachment mime type: text/plain → image/png
Ben, Brian, Can you comment on why gnomestripe styles autocomplete widgets differently from winstripe (specifically difference #1 from comment 11 in this bug)? We're trying to figure out if we can change gnomestripe to style such widgets the same way winstripe (and pinstripe, I think) style them.
Ben, Brian, Can you comment on why gnomestripe styles autocomplete widgets differently from winstripe (specifically difference #1 from comment 11 in this bug)? We're trying to figure out if we can change gnomestripe to style such widgets the same way winstripe (and pinstripe, I think) style them.
(In reply to comment #13) > Also, when I tested the ThreeDShadow stuff locally, it looked odd, because the > endcap images have a particular color/opacity, and "ThreeDShadow" didn't match > it; so where the endcap images met the text boxes, things looked wrong. > Instead of using ThreeDShadow we should presumably be using the same > color/opacity as the endcap images. Would be no problem on the trunk, but Gecko 1.8 doesn't support rgba(). But I wouldn't do that anyway.
Blocks: 349184
Attached patch patch v3: minimal fix (deleted) — Splinter Review
Ok, here's the minimal fix as called for by comment 13. I also replaced ThreeDShadow with the color we're currently using to style the borders of the non-native search bar: #96969D. With this patch, the location bar's textbox looks non-native, but its dropmarker still looks native, i.e. ugly.
Attached image screenshot of v3 in action on Linux (deleted) —
(In reply to comment #19) > Created an attachment (id=237401) [edit] > screenshot of v3 in action on Linux This means the dropdown will look "next to" the location bar rather than "inside", right? bug 263750 comment 7 suggests that maybe the way out is to give the location bar a -moz-appearance: textfield. Although maybe at this point I'm really reversing what I said in comment 13... Really, it seems like a hand-wavingly ideal solution would: * Make the Linux and Windows location bars be structured the same * Make sure that structure is fully styled by us with the border size/color you specified in your patch, etc. so there are no strange bevels that appear * Not screw with random other non-location-bar bars like the "Open Web Location" thing But maybe I'm wrong about that last point, and your patch v2 (except using the hardcoded colors from v3) is in fact the way to go, since presumably it handles points 1 and 2...
> This means the dropdown will look "next to" the location bar rather than > "inside", right? Right. > bug 263750 comment 7 suggests that maybe the way out is to > give the location bar a -moz-appearance: textfield. Although maybe at this > point I'm really reversing what I said in comment 13... It's partly a reversal of comment 13, except that we could do this just for the location bar rather than for autocomplete widgets generally. But while this would get the dropmarker inside the location bar rather than next to it, it wouldn't make the location bar non-native. It would just change which widget is being natively styled. > Really, it seems like a hand-wavingly ideal solution would: > * Make the Linux and Windows location bars be structured the same > * Make sure that structure is fully styled by us with the border size/color > you specified in your patch, etc. so there are no strange bevels that appear > * Not screw with random other non-location-bar bars like the "Open Web > Location" thing > > But maybe I'm wrong about that last point, and your patch v2 (except using > the hardcoded colors from v3) is in fact the way to go, since presumably it > handles points 1 and 2... Yes, patch v2 w/hardcoded colors from v3 does indeed handle points 1 and 2. Given that Pam has already landed her dropmarker fixes into toolkit/themes/winstripe/global/autocomplete.css, I suspect we're better off making gnomestripe reuse winstripe style for autocomplete widgets, since it means simpler code with fewer regressions which are easier to track down and fix. But if necessary, I can make a patch which applies the fixes only to the location bar.
(In reply to comment #20) > But maybe I'm wrong about that last point, and your patch v2 (except using the > hardcoded colors from v3) is in fact the way to go Could you post a screenshot, showing how ThreeDShadow looks odd with your setting? At least in attachment 237364 [details], the color difference would never bother me, since the endcaps are relatively small. Contrary, I can easily imagine a bluish border for the whole address and search bars looking out of place in terms of OS integration.
Here's the complete fix from v2 but using the same colors that the search bar is currently using (presumably it's the same color as the border on the Go and Search buttons). This patch also fixes a regression in v2 that drew borders both to the left and to the right of the search textbox (the current theme only draws a border to the right of the textbox). And I throw in a fix for bug 350788, so that the border switches to the left of the textbox in RTL mode.
Attachment #237361 - Attachment is obsolete: true
Attachment #237432 - Flags: review?(mconnor)
Attachment #237361 - Flags: review?(mconnor)
The current patch up for review in this bug will also fix bug 350788 and will partly fix bug 351618, so making those bugs depend on this one.
Blocks: 350788, 351618
Also note that the Go button doesn't actually use a color like #96969D but a 80% opaque grey.
(In reply to comment #26) > Also note that the Go button doesn't actually use a color like #96969D but a > 80% opaque grey. That's more what I'm worried about; we should make the box borders do whatever the Go border is doing, which is definitely not ThreeDShadow, but sounds like isn't 96969D either.
(In reply to comment #27) > That's more what I'm worried about; we should make the box borders do whatever > the Go border is doing, which is definitely not ThreeDShadow, but sounds like > isn't 96969D either. The Go button should rather try to match the theme, and I think darkening the toolbar background is the best we can do. On the trunk, using a ThreeDShadow CSS border and border-radius for the Go button might be an option, but without anti-aliasing that's ugly. Now since the Go button's does darken the toolbar background, there is no single color you can apply to the urlbar. You would need rgba() for that.
(In reply to comment #28) > Now since the Go button's does darken the toolbar background, there is no > single color you can apply to the urlbar. You would need rgba() for that. Just for the record, I think using rgba() would be far from perfect, too. It would match the Go button, but also inherit its problems with dark OS themes where the border disappears.
(In reply to comment #28) > On the trunk, using a ThreeDShadow CSS border and border-radius for the Go > button might be an option, but without anti-aliasing that's ugly. Hmm ... maybe worth a try even without anti-aliasing, if the radius is not too big. 1 or 3px could work, depends on the image.
Would be great if it always looked like this. Unfortunately, resizing the window makes the border go crazy.
Comment on attachment 237432 [details] [diff] [review] patch v4: complete fix with the right color Looks good from here. There's a potential for color mismatch given the slight opacity on the image borders, but we may get new images to fix that.
Attachment #237432 - Flags: review?(mconnor)
Attachment #237432 - Flags: review+
Attachment #237432 - Flags: approval1.8.1+
Whiteboard: [Fx2 theme change][checkin needed (1.8 branch)]
End cap borders are much, *much* lighter now. Again, I advocate ThreeDShadow.
Fix checked in to the branch. Note that mconnor's rollup checkin earlier today included the browser.css and searchbar.css fixes in the patch for this bug, so my checkin was just for the autocomplete.css and jar.mn fixes. Also note that you have to do a clobber build for these fixes to take effect.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [Fx2 theme change][checkin needed (1.8 branch)] → [Fx2 theme change]
Depends on: 355756
Depends on: 352678
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: