392693 - Inconsistent arrow button size and entry field width in Add Bookmark dialog.

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[David Tholen]

User-Agent:       Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.8.1) Gecko/20061025 Firefox/2.0
Build Identifier: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.8.1) Gecko/20061025 Firefox/2.0

When I click on Bookmarks --> Bookmark This Page... a small Add Bookmark window appears with two entry fields and a downward pointing arrow to the right of the "Create in:" entry field.  That arrow appears inside of a square-looking button, but if I position the mouse pointer over it, the button suddenly becomes rectangular in shape, while the two entry fields get slightly wider.  Move the mouse pointer away, and the button goes back to the original square shape while the entry fields return to their previous widths.

Reproducible: Always

Steps to Reproduce:
1. Click on Bookmarks.
2. Click on Bookmark This Page...
3. Move mouse pointer over down arrow button.
Actual Results:  
Entry fields get wider, button gets smaller and more rectangular.

Expected Results:  
Should be no change in user interface.  Earlier builds of Firefox didn't do this.

Comment 2

[Walter Meinl]

Once in a while I stumbled also over this (I think it was ever since the 2.0
series started) but somehow have forgotten to file a bug about it.
The problem with the small arrow on the right side is still seen in the trunk
builds.
But I see also a little difference between 2.0 and trunk: in addition in the
2.0 builds the arrow for the dropdown list is missing and the box is smaller
than in trunk builds. So there was obviously a partial check-in for the trunk. Maybe that helps to find the code to change

Comment 3

[Peter Weilbacher]

Although I do not see the problem on Windows and Linux I have no clue why this should be OS/2 specific. Buttons are still using the same theme style as on Windows...

Comment 4

[Walter Meinl]

(In reply to comment #3)
> Although I do not see the problem on Windows and Linux I have no clue why this
> should be OS/2 specific. Buttons are still using the same theme style as on
> Windows...
> 
Peter, actually the "flickering" is caused by different arrow-sizes. The arrows you introduced with pmstripe contain some transparent background, therefore the surrounding field is big. When you click with the mouse or hover the mouse over the arrow, pmstripe does not contain an arrow-dn-hov.gif, thus, the winstripe arrow-dn-hov.gif is used. As it is smaller than the pmstripe arrow-dn.gif, the surrounding "button" field gets small.
I've reduced the pmstripe gif's to contain only a minimum of transparent background, then the field was small already when the dialog came up. However, the hovering arrows of winstripe are still 1 point smaller than the pmstripe arrows and I see still a minimal flickering.
I got it stable when I copied the arrow-*.gif to arrow-*-hov.gif and included the *-hov.gifs in jar.mn.
Now, it's only the question, if we want to have a big "button" or a small one. For the big one you can reuse the current arrows. For the small button, the transparent background from the current arrows would have to be removed before. I did this already and can send you these arrows per private mail (it's to annoying to attach them one by one here)

Comment 5

[Peter Weilbacher]

The size of the arrows that I added to pmstripe has to stay, I think, because it otherwise messes up the scrollbar arrows again. I remember that I had a hard time to get it to agree for all those...

I guess we could copy over a few more and change to the same size. But how do we find out if that wouldn't mess them up somewhere else?

Comment 6

[Walter Meinl]

(In reply to comment #5)
> The size of the arrows that I added to pmstripe has to stay, I think, because
> it otherwise messes up the scrollbar arrows again. I remember that I had a hard
> time to get it to agree for all those...
> 
> I guess we could copy over a few more and change to the same size. But how do
> we find out if that wouldn't mess them up somewhere else?
> 
To be honest, I do not see any differences, if I use the bigger ones or the smaller ones except the add bookmarks dialog here. Scrollbars look the same and stay the same with either arrow. Probably, your fruitful work in xulbars.css protect us from hosed arrows as they looked like for so long time before you made the pmstripe overrides. And from a point of logic (if this counts) I would expect with equal sized arrows less problems, maybe no problems as this one here seems to be the only one after about 1 year people used your artwork.
So probably the safest way would be to duplicate all arrow-*.gif to *-hov.gif.(in Windows they use also the same arrows for the hovering).

Comment 7

[Peter Weilbacher]

OK, after a real test I can confirm that cutting the arrows doesn't create a centering problem for the scrollbars. But I can only cut them to a size of 6x3px while the disabled ones have 5x3px, so that wouldn't really help. When cutting down the size of the arrows I also find that the button in the bookmarks dialog becomes very small while I think it should have the height of the box on the left of it. So I propose to leave the arrows as is but instead add them to the JAR file twice, once under the *-hov.gif name. Patch upcoming.

Btw, the problem isn't there any more on the trunk because that now got a different dialog, where probably the buttons are sized by something else (padding? borders? didn't investigate in detail).


Did anyone find out why the down arrow on the dropdown button in the field is not showing? It's a standard xul:dropmarker which points to chrome://global/skin/arrow/arrow-dn.gif and that I can load without problems...

Comment 8

[Peter Weilbacher]

Attachment: proposed fix

Comment 9

[Walter Meinl]

(In reply to comment #7)
> OK, after a real test I can confirm that cutting the arrows doesn't create a
> centering problem for the scrollbars. But I can only cut them to a size of
> 6x3px while the disabled ones have 5x3px, so that wouldn't really help.
Yes, the disabled are smaller (windows size). I copied the active arrows to disabled and then loaded them with gimp, the broadest line is 6 px, I removed pixel 2 and 5, from the 4 px line, I removed the pixels 2 and 3, et voila, same size as the active (6x3) with the look of disabled. You see of course the "pixel work", but you see it also with the 5x3 arrow, e.g. in the location bar next to the back or forward buttons, and more importantly also in pm applications when you scroll down to the end of a page in e.exe for instance.
 
> When cutting down the size of the arrows I also find that the button in the
> bookmarks dialog becomes very small while I think it should have the height of
> the box on the left of it.
That was the behavior in every build of ff since winstripe was chosen as the new theme. I personally like also the bigger box. Now an academic question, why has windows with it's small arrows a big box? Native theming? on linux it is small.
> Btw, the problem isn't there any more on the trunk because that now got a
> different dialog, where probably the buttons are sized by something else
> (padding? borders? didn't investigate in detail).
That's not completely true, the box doesn't change its size, when you hover over it, that is different, but when you press the buttons in the new starring dialog, you see that in the pressed state the box gets small again.
> 
> Did anyone find out why the down arrow on the dropdown button in the field is
> not showing? It's a standard xul:dropmarker which points to
> chrome://global/skin/arrow/arrow-dn.gif and that I can load without problems...
> 
I'm pretty close to find it out, hopefully this evening

Comment 10

[Peter Weilbacher]

Of course you can edit the GIFs to have the same size but then they are not the same as the PM counterparts any more. For those, the disabled one has a 5px wide base while the normal one has a 6px base. I don't really want to give that up.

Comment 11

[Walter Meinl]

(In reply to comment #10)
> For those, the disabled one has a 5px
> wide base while the normal one has a 6px base. I don't really want to give that
> up.
> 
Was just a suggestion in case the disabled would make trouble, but I don't see any troubles from the disabled at the moment, so leaving them as they are is probably fine.
> Did anyone find out why the down arrow on the dropdown button in the field is
> not showing? It's a standard xul:dropmarker which points to
> chrome://global/skin/arrow/arrow-dn.gif and that I can load without problems...
yep, here we go. I found that on the trunk (with --disable-places) the dropdown button was missing also until bug348614 was checked in. But this checkin was a rather big patch creating lots of new files and doesn't apply w/o adjustment on the branch today. However, there was another bug filed (from MKaply) because of this problem, but was never checked in. It's bug314962. The fix has already r+ and it applies cleanly on the branch (checked out 2 or 3 days ago). Probably that is what we should take for the branch as it restores the dropdown marker, doesn't affect too much other files. Moreover, I can confirm that linux builds are ok with this patch.

Comment 12

[Peter Weilbacher]

wuno, thanks again for the detective work! So let's get this patch reviewed and checked in. While doing that we can try to reactivate bug 314962.

Comment 13

[Walter Meinl]

Peter, I applied your patch to firefox trunk (omitting the last line) https://bugzilla.mozilla.org/attachment.cgi?id=279812 and it works very well with the new starred pane and the reorganized places bookmark manager (arrows are in the center of a big button). So how about checking this in? 2.0.0.8 comes out soon

Comment 14

[Peter Weilbacher]

OK, while waiting for the review I had forgotten about this.

I thought trunk was fine even without this patch? So is it really needed for both branch and trunk?

Comment 15

[Walter Meinl]

I'd add it also for the trunk, cause the respective buttons of the new panes in bookmark manager or the starred pane shrink in the moment you press (not when you hover over) the button to a small boxed shape. It's less obvious, but for consistency it would be probably good.

Comment 16

[Peter Weilbacher]

Comment on attachment 279812 [details] [diff] [review]
proposed fix

r+ by Walter Meinl through testing.

Comment 17

[Peter Weilbacher]

Checked in to branch and trunk.

Comment 18

[Reed Loden [:reed]]

This missed the 1.8.1.8 train by ~15 minutes.