Closed Bug 654916 Opened 14 years ago Closed 14 years ago

To/Cc Popups are uneven for first and last rows

Categories

(Thunderbird :: Message Compose Window, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 5.0b1

People

(Reporter: davida, Assigned: Paenglab)

References

Details

Attachments

(6 files, 1 obsolete file)

Attached image screenshot (deleted) —
See attached screenshot. Notice different size of both Cc lines
this is with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0a2) Gecko/20110504 Thunderbird/3.3a4pre
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Richard, I'm reopening this to treat it separately from the menu-width issue in bug 654426. After having a closer look, I could reproduce this in both Linux and Windows 7, the first and last row have buttons with a reduced height. Also, if you move the splitter down to increase the number of rows, then you will notice that the previously shrunk button expands after a certain height of the listbox has been reached. Also, spacing of the rows changes in the same way, thus those two effects are somehow connected.
Status: RESOLVED → REOPENED
OS: Mac OS X → All
Hardware: x86 → All
Resolution: DUPLICATE → ---
Summary: To/Cc Popups look uneven on os x → To/Cc Popups are uneven for first and last rows
Version: unspecified → Trunk
Attached image on Windows 7 (deleted) —
I've selected the "Reply-to:" heading here on purpose as you can nicely see the different distances between the lowest pixel of the 'p' and the lower borders of the menu buttons. This is Mozilla/5.0 (Windows NT 6.1; WOW64; rv:5.0a2) Gecko/20110506 Thunderbird/3.3a4pre on Windows Classic.
Blocks: 642163
Severity: normal → trivial
Status: REOPENED → NEW
While we're tweaking css here, on the mac at least the focus rings on the To/Cc menu are clipped to the left. (shift-tab to select it, likely requires telling OS X to enable tabbing to non-text-fields in the system prefs).
Please can you attach a screenshot with the focus ring?
Maybe move any Mac-related issues to the platform-specific bug 654426 and keep this bug here for the cross-platform issues with that listbox?
The entire row for the first and last rows is one pixel short, which I'm guessing is because of the listcols element having a border. One way to fix this would be to add: #addressingWidget listboxbody { margin: 1px; }
I found, the problem was the removing of the listitem borders. I don't know, if this then triggers rounding errors. Your proposal works, but makes the listitem smaller (22px instead 24px height). I had already an other solution. I'll check, if I can combine both.
Have a look at the comparison in bug 654686 attachment 529968 [details]. The row spacing increased between the old and the new implementation, thus if you'd go with 22px, this would restore the previous row height which should be sufficient (thus saving some vertical space in the process).
Attached image screenshot of focus ring (deleted) —
screenshot w/ focus ring. i don't know if it's platform specific.
Attached patch Patch fixing this issue (hopefully on OSX) (obsolete) (deleted) — Splinter Review
#addressingWidget listboxbody { margin: 1px; } did the fix. But why does a margin on the listboxbody solve a problem on childs (listitem, listcell)? This patch needs Bug 643262 and Bug 654686 applied. If this patch is earlier ready to land, I can update. I gave for OSX 1px margin also for left and right in the hope, this solves the focus ring problem. OSX isn't tested because I have no Mac.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #530826 - Flags: review?(bwinton)
(In reply to comment #12) > OSX isn't tested because I have no Mac. I'll do that, because I soon have a Mac build with this patch.
Attached image Screenshot, Mac with patch (deleted) —
With this patch and the patch from Bug 654426, it looks now like this on Mac.
Nomis101 thank you for testing. It looks the menulists have now the same height, but are aligned on top and not centered. I hope :andreasn can this and the still appearing truncating address in Bug 654426.
Attached image OSX with the patch (I think). (deleted) —
And for completion, here's the focus ring on the top, middle, and bottom dropdowns.
Regarding the focus ring: I checked the images again. It looks like the menulists itself have a glow effect. Is this focus ring still needed?
Comment on attachment 530826 [details] [diff] [review] Patch fixing this issue (hopefully on OSX) Review of attachment 530826 [details] [diff] [review]: ----------------------------------------------------------------- I don't see any glow effect on OS X. And I'm not sure whether we're adding the focus ring, or whether the OS is… Either way, it might be solved if we messed with the padding instead of the margin… But handling it in bug 654426 makes some sense, since it seems to be Mac-specific. Other than that, this definitely fixes the problem (at least on OS X), so I think going with this, and doing the other work in followup bugs makes sense. r=me. ::: mail/themes/gnomestripe/mail/compose/messengercompose.css @@ +442,5 @@ > background-color: transparent; > } > > +#addressingWidget listboxbody { > + margin: 1px; Why do all the other themes use "1px 0", but pinstripe uses "1px"? Is it a typo, or intentional?
Attachment #530826 - Flags: review?(bwinton) → review+
(In reply to comment #18) > I don't see any glow effect on OS X. And I'm not sure whether we're adding > the focus ring, or whether the OS is… As :davida in Comment 5 wrote: (shift-tab to select it, likely requires telling OS X to enable tabbing to non-text-fields in the system prefs) it's not enabled as standard. I found in the css file this: .aw-menulist:focus { outline: 2px solid -moz-mac-focusring; outline-offset: -2px; } This looks as the creator of this focus rings, which now are no more used, because the aw-menulist has no more none appearance. > ::: mail/themes/gnomestripe/mail/compose/messengercompose.css > @@ +442,5 @@ > > background-color: transparent; > > } > > > > +#addressingWidget listboxbody { > > + margin: 1px; > > Why do all the other themes use "1px 0", but pinstripe uses "1px"? Is it a > typo, or intentional? I hoped this solves the problem with the focus ring being cut on the left, but didn't work. Because the other patches are not reviewed, I'm creating a new patch without this dependency and give the same margin as the other themes. Additionally I remove the focus ring, if it's okay. Does this patch need a new review because of the changes?
Attached patch Patch for check-in (deleted) — Splinter Review
Because of the changes in Comment 19 mentioned asking again review.
Attachment #530826 - Attachment is obsolete: true
Attachment #530925 - Flags: review?(bwinton)
Comment on attachment 530925 [details] [diff] [review] Patch for check-in Review of attachment 530925 [details] [diff] [review]: ----------------------------------------------------------------- Yes, I see what you mean now. I like the glow _way_ more. It's still cut off a little on the left (and the top of the first one), but that sounds like it can get handled in the other bug. r=me, and thanks! :)
Attachment #530925 - Flags: review?(bwinton) → review+
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 14 years ago14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: