191073 - Classic Theme: personal toolbar has a white vertical line on the left

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Christian Weisel]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030128
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030128

On the classic theme: The home button has a white 1px width vertical line on the
left side.

Reproducible: Always

Steps to Reproduce:
1. Change to classic theme
2. Activate the home button (if it's not there)
3. Look at the button

Actual Results:  
You will the the extra 1px width line on the home button.

Expected Results:  
There should be a white 1px seperator line between the colapse buttons an the
toolsbars. But not that extra white line of the home button

Comment 1

[José Jeria]

Attachment: Screenshot

Comment 2

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030128
I´m seeing this line also if Home button and/or Bookmarks button are deactivated,
I´ve got a bookmarks folder in the personal toolbar.

Comment 3

[Owen Marshall (Not reading bugmail)]

CC'ing marlon, who does icon issues (right?)

Comment 4

[Michael Lefevre]

confirming (using 2003020508/win2k)

Comment 5

[José Jeria]

Is this still valid?

Comment 6

[Andreas Kunz]

Yes. It still looks like in your screenshot.

Comment 7

[José Jeria]

This is a very trivial bug, but also very visible one. Nominating for 1.7a, so
it hopefully gets some attention.

Comment 8

[Andreas Kunz]

Attachment: Patch: prefixhidden="true" for Personal Toolbar

This fixes it: set the <toolbar> attribute: prefixhidden="true"

toolbar-prefix is an additional hbox that was added for bug 134345 which did
some work for adding a sidebar open/close button to the Personal Toolbar in
_Netscape_ (Mozilla does not have it).

The added attribute comes from
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/toolbar.xml#125

and is used in the same way as here for the sidebar in
http://lxr.mozilla.org/seamonkey/source/xpfe/components/sidebar/resources/sidebarOverlay.xul#67


I'm not sure why it the Navigation Toolbar does not show that light line
although not hiding it. According to DOM Inspector it is there as well...

Comment 9

[Andreas Kunz]

Umm... now Modern theme has NO light line there. Not good, will look into it.

Comment 10

[Andreas Kunz]

Attachment: Revised patch, works for Modern and Classic

Ok, I found the problem: the author of the bug 134345 patch obviously was using
Modern theme and after adding the toolbar-prefix box he saw exactly what this
bug is about: a doubled light vertical line.

But instead of hiding his box when not needed, he deleted the other light
vertical line (the legitimate one): the left border of the toolbar holder (see
diff from version 1.76 to 1.77 of navigator.css). Unfortunately he only did it
for Modern theme, so this bug here remained.

This patch now adds the left border back in Modern (giving it the same color
code it had before). And of course it still hides the toolbar-prefix box as the
first version already did.

I tested it and I guess the only possible issue is with the Netscape
distribution that optionally has a sidebar toggle button in the personal
toolbar. 


Please only read the rest of this comment if you care for Netscape 7.
[I installed Netscape 7.1 and what I first saw is that it is terribly broken:
Modern theme lacks the light vertical line when there is no sidebar button and
Classic theme has one such line too much between the sidebar button and the
bookmarks button when there is one.
My patch would add such a line there also for Modern theme, a fix (that also
would fix the already broken classic theme) would be to add a dark line to its
left so the sidebar button gets a properly separated button.
So my fix does not break Netscape much more than it already is - and the fix
would be trivial.
Uh, I forgot: Netscape is dead...
So this is only important with respect to other distributions that have a
sidebar button and shows that my fix is no problem for them.]

Comment 11

[Andreas Kunz]

Comment on attachment 138260 [details] [diff] [review]
Revised patch, works for Modern and Classic

Requesting r= from Neil.

Comment 12

[José Jeria]

Great! Will this make this into 1.6?

Comment 13

[Andreas Kunz]

Comment on attachment 138260 [details] [diff] [review]
Revised patch, works for Modern and Classic

Requesting sr= from alecf.

José: well, when this patch has r= and sr= you may request approval for 1.6 (if
it is not released by then). I certainly would consider its risk being low, but
others have to decide.

Comment 14

[[e-mail]]

Comment on attachment 138260 [details] [diff] [review]
Revised patch, works for Modern and Classic

Ugh, this is such a mess, I would be quite happy to see the toolbarprefix hacks
removed completely...

Comment 15

[Andreas Kunz]

That's what I thought...
But for now this patch should suffice, I don't think I'm able to cleanly remove
all this stuff. Nevertheless it might be a rewarding cleanup.

Comment 16

[Alec Flett]

Comment on attachment 138260 [details] [diff] [review]
Revised patch, works for Modern and Classic

sr=alecf

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in on trunk:
Checking in xpfe/browser/resources/content/navigator.xul;
/cvsroot/mozilla/xpfe/browser/resources/content/navigator.xul,v  <--  navigator.xul
new revision: 1.411; previous revision: 1.410
done
Checking in themes/modern/navigator/navigator.css;
/cvsroot/mozilla/themes/modern/navigator/navigator.css,v  <--  navigator.css
new revision: 1.84; previous revision: 1.83
done

Comment 18

[Andreas Kunz]

fixed for 1.7 trunk
Currently _I_ have no intention to get this into 1.6.