462663 - Height of tags edit field shrunken since landing of tags autocomplete feature

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: shrunken height of tags edit control

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081101 Minefield/3.1b2pre ID:20081101032525

After the checkin of the tags autocomplete patch the tags edit field has changed its height. Now it looks really stocky when comparing to the other fields. See the attached screenshot.

It should have the same height as all the other fields.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I forgot to say that it doesn't occur on OS X.

Comment 2

[Peter Lairo]

Attachment: WFM in WinXP

Not seeing this in

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081101 Minefield/3.1b2pre

Comment 3

[Peter Lairo]

Attachment: Scrollbar too narrow?

But the vertical scrollbar does seem more narrow than the vertical scrollbar for the folder selection.

Comment 4

[Peter Lairo]

Err, I meant "wider" (the folder one seems more narrow). :-[

Comment 5

[Peter Lairo]

Attachment: New bug: Folder box too small

BTW: The expanded folder box is not tall enough (i.e.: *much* too short). Ideally, the user could expand it (via grippy) and Firefox would remember the new size. Is there a bug on that yet?

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This bug is about the height of the tags field which regressed two month ago. Please don't spam it with other issues. Thanks.

(In reply to comment #5)
> BTW: The expanded folder box is not tall enough (i.e.: *much* too short).
> Ideally, the user could expand it (via grippy) and Firefox would remember the
> new size. Is there a bug on that yet?

Yes, AFAIR there is one. But I cannot find it currently.

Comment 7

[Marco Bonardo [:mak]]

Attachment: patch

reset default textfield appearance, even if this is an autocomplete field

Comment 8

[Marco Bonardo [:mak]]

confirmed valid on gnomestripe too, pinstripe is correct since default textfield padding is 0

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 347302 [details] [diff] [review]
patch

Seems like there's a "padded" class in autocomplete.css specifically for this use case. So we should use it here. (And if it's not quite right, e.g. regarding the cursor or for pinstripe's padding-less textboxes, we should fix that in autocomplete.css.)

Comment 10

[Marco Bonardo [:mak]]

Dao, i see the padded class but we have request that all fields have same height (bug 433055), using .padded would add some padding, but not the same as other textboxes, so it would still be different.
So, are you saying i should fix autocomplete.css padded class to use the same padding as normal textfields?
Instead on pinstripe autocomplete.css padded class has a padding, while normal textfields don't appear having one (see widget textbox.css)

About the cursor, i can't tell why autocomplete.css defines the cursor as default on autocomplete textboxes

Comment 11

[Dão Gottwald [:dao]]

(In reply to comment #10)
> Dao, i see the padded class but we have request that all fields have same
> height (bug 433055), using .padded would add some padding, but not the same as
> other textboxes, so it would still be different.
> So, are you saying i should fix autocomplete.css padded class to use the same
> padding as normal textfields?

Yes... please fix autocomplete.css like this:

textbox:not(.padded) {
  padding: 0;
  cursor: default;
}

While you're fixing that, please remove #browserHomePage from winstripe's and gnomestripe's preferences.css (in browser/).

Comment 12

[Marco Bonardo [:mak]]

Attachment: patch

So, this is acting like we discussed on IRC, only change is that
textbox:not(.padded) > .textbox-input-box
was removing the margin from the locationbar too, so i changed it to a more generic
textbox:not(.padded) .textbox-input-box

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 347359 [details] [diff] [review]
patch

You probably shouldn't remove /* Used by autocomplete widgets that don't have an icon. Gross. -dwh */ entirely, because it's really an ugly way to solve this and the class name isn't quite obvious.

Comment 14

[Marco Bonardo [:mak]]

Attachment: patch

added back comment

Mike, do we want to take this minor cleanup for the beta?

Comment 15

[Dietrich Ayala (:dietrich)]

Comment on attachment 347529 [details] [diff] [review]
patch

you need r+ from a toolkit peer for changes to autocomplete. gavin, mconnor, enn are valid candidates for changes here, iirc.

Comment 16

[Marco Bonardo [:mak]]

Comment on attachment 347529 [details] [diff] [review]
patch

moving review to gavin

Comment 17

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 347529 [details] [diff] [review]
patch

>-#browserHomePage {

>-  background-color: -moz-Dialog;

I was pretty curious about why this was here to begin with - looks like it was landed accidentally as part of bug 309140.

>+/* .padded is used by autocomplete widgets that don't have an icon. Gross. -dwh */

Gross indeed! We should get a bug filed on just doing the right thing and making .padded unnecessary...

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 347529 [details] [diff] [review]
patch

a=beltzner, this should block as well so we don't pooch theme developers :(

Comment 19

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/5bc95a37a77b

Comment 20

[Marco Bonardo [:mak]]

(In reply to comment #17)

> >+/* .padded is used by autocomplete widgets that don't have an icon. Gross. -dwh */
> 
> Gross indeed! We should get a bug filed on just doing the right thing and
> making .padded unnecessary...

filed Bug 464450

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #19)
> http://hg.mozilla.org/mozilla-central/rev/5bc95a37a77b

This fixes the mentioned issue on Windows XP. But Vista is still affected. Looks like the patch misses the CSS update for Aero. I'll attach a screenshot which shows the issue with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081114 Minefield/3.1b2pre ID:20081114034305

=> Reopen

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Tags field on Vista

Comment 23

[Marco Bonardo [:mak]]

there's no special file for aero and i see padding in the field, that sounds more like bug 433055

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

You are right. Lets move it over to bug 433055. Marking this bug as verified.

Comment 25

[Marco Bonardo [:mak]]

as an additional check i've compared with current branch, and your screen looks the same, so that issue has not been regressed by tag autocomplete.

Comment 26

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv