Text fields in bookmark properties should have the same height

VERIFIED FIXED in Firefox 3.1b2

Status

()

Firefox
Theme
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: Ehsan, Assigned: dao)

Tracking

({polish})

Trunk
Firefox 3.1b2
x86
Windows Vista
polish
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [polish-easy] [polish-visual][polish-p2], URL)

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
For a sample of the problem, please check out <https://bugzilla.mozilla.org/attachment.cgi?id=320156>.  All single like text boxes in those dialogs should have the same height.
Whiteboard: [polish-easy] [polish-visual]

Comment 1

10 years ago
mh, i suppose the different height is due to the name picker used for microsummaries
There are a lot of other instances of text fields being different heights on Vista, which cropped up when we switched to the native appearance. cc'ing vlad in case he knows more.
(Assignee)

Updated

10 years ago
Assignee: nobody → dao
(Assignee)

Comment 3

10 years ago
Created attachment 348483 [details] [diff] [review]
places theme patch

This is ugly. IMHO you should use a real textbox when there are no microsummaries.
Attachment #348483 - Flags: review?(mak77)
(Assignee)

Updated

10 years ago
Attachment #348483 - Attachment description: places → places theme patch

Comment 4

10 years ago
this is cool, but there is still a jumpy behaviour on the namepicker when we HAVE a microsummary, the text is missing padding on the left and has too much padding on the top... so if you move in the Library from an item that has microsummary to an item that does not have one, the namepicker jumps.

You can test with one of these sites: https://wiki.mozilla.org/Microsummaries/Sites
(Assignee)

Comment 5

10 years ago
(In reply to comment #4)
> this is cool, but there is still a jumpy behaviour on the namepicker when we
> HAVE a microsummary, the text is missing padding on the left and has too much
> padding on the top...

This might be harder to fix. I suggest we file a new bug under Toolkit:Theme, as it affects all editable menulists.
(Assignee)

Comment 6

10 years ago
(In reply to comment #3)
> This is ugly. IMHO you should use a real textbox when there are no
> microsummaries.

Btw, faking the textbox also means that you're lacking the fix for bug 288194.

Comment 7

10 years ago
Comment on attachment 348483 [details] [diff] [review]
places theme patch

>diff --git a/browser/themes/winstripe/browser/places/editBookmarkOverlay.css b/browser/themes/winstripe/browser/places/editBookmarkOverlay.css
>--- a/browser/themes/winstripe/browser/places/editBookmarkOverlay.css
>+++ b/browser/themes/winstripe/browser/places/editBookmarkOverlay.css
>@@ -79,38 +79,33 @@
> 
> /**** name picker ****/
> 
> /* Make the microsummary picker look like a regular textbox instead of 
>  * an editable menulist when no microsummaries are available.
>  */
> #editBMPanel_namePicker[droppable="false"] {
>   /* These rules come from the textbox element in textbox.css. */
>-
>-  /* Normal editable menulists set this to "none". */
>-  -moz-appearance: textfield;
>+  -moz-appearance: textfield; /* Normal editable menulists set this to "menulist". */
>   cursor: text;
>-
>+  margin: 2px 4px;

is this margin really needed?


for menulists i see in toolkit menulist.css

127 /* ::::: editable menulists ::::: */
128 
129 .menulist-editable-box {
130   padding-top: 3px;
131   padding-bottom: 3px;
132   -moz-padding-start: 2px;
133   -moz-padding-end: 0px;
134 }
so yes i agree with your suggest to file a toolkit theme bug.

over to dietrich.
Attachment #348483 - Flags: review?(mak77) → review?(dietrich)
(Assignee)

Comment 8

10 years ago
(In reply to comment #7)
> >+  margin: 2px 4px;
> 
> is this margin really needed?

It's the margin that real textboxes have as well per textbox.css.

Comment 9

10 years ago
(In reply to comment #8)
> (In reply to comment #7)
> > >+  margin: 2px 4px;
> > 
> > is this margin really needed?
> 
> It's the margin that real textboxes have as well per textbox.css.

yes but from menulist.css:
menulist {
49   -moz-appearance: menulist;
50   margin: 2px 4px;

it's already defined...

hwv i can see the point in having a real textbox there, that could be the right fix, maybe now or for a followup, dietrich can elaborate over that.
Attachment #348483 - Flags: review?(dietrich) → review+
Comment on attachment 348483 [details] [diff] [review]
places theme patch

using the proper widget would be better, but this is an acceptable interim solution. r=me w/ comment #9 resolved.
(Assignee)

Comment 11

10 years ago
(In reply to comment #10)
> r=me w/ comment #9 resolved.

by that you mean removing the redundant margin declaration?
(In reply to comment #11)
> (In reply to comment #10)
> > r=me w/ comment #9 resolved.
> 
> by that you mean removing the redundant margin declaration?

i guess so since he talked about interim solution. We should also file a Bookmarks&History bug to do the correct field replacement in future and a toolkit theme bug for menulists padding.
(Assignee)

Updated

10 years ago
Attachment #348483 - Flags: approval1.9.1b2?
Attachment #348483 - Flags: approval1.9.1b2? → approval1.9.1b2+
Comment on attachment 348483 [details] [diff] [review]
places theme patch

a191b2=beltzner
(Assignee)

Comment 14

10 years ago
http://hg.mozilla.org/mozilla-central/rev/71a967ce2879
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1b2
(Assignee)

Comment 15

10 years ago
bug 465718 and bug 465719
Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081120 Minefield/3.1b2pre ID:20081120033739

Now the drop down is the only element which is too high. Mostly it will depend on the shown icon on the left side. Could we shrink this too, so we have the same height as the buttons at the right side? If there is no bug about it, I could file a new one - at least if it can be fixed.
Status: RESOLVED → VERIFIED
This bug's priority relative to the set of other polish bugs is:
P2 - Polish issue that is in a secondary interface, occasionally encountered, and is easily identifiable.

Considering bookmarking a secondary UI, and this was obviously wrong.
Whiteboard: [polish-easy] [polish-visual] → [polish-easy] [polish-visual][polish-p2]
You need to log in before you can comment on or make changes to this bug.