Note: There are a few cases of duplicates in user autocompletion which are being worked on.

odd "left" and "right" arrow images in the "Previous" / "Next" buttons in the table properties dialog

RESOLVED FIXED in Thunderbird 19.0

Status

MailNews Core
Composition
--
trivial
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: aceman)

Tracking

Trunk
Thunderbird 19.0
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

odd "left" and "right" arrow images in the "Previous" / "Next" buttons in the table properties dialog

here comes a screen shot.
Created attachment 255974 [details]
screen shot

Comment 2

11 years ago
It's super easy to remove these styles from the two buttons by altering editorDialog.css. I agree they look weird. It looks like they were intentionally put on those two buttons back before the dawn of time though and were added intentionally. I think the composer dialog is using those two buttons to let you navigate the underlying table and they use the arrows to indicate to you which way the selection is going to move if you push the button. The icons even change if you have a row selected vs. a cell.

Not sure what the right thing to do is.
> The icons even change if you have a row selected vs. a cell.

wow, you are right.  Do you see a down arrow on the "next" button when you do "row"?  I don't.  Do you want a spin off bug on that?

> Not sure what the right thing to do is.

me neither.  but at the very least, maybe add some padding to separate the image from the text in the button?
(In reply to comment #3)
> wow, you are right.  Do you see a down arrow on the "next" button when you do
> "row"?  I don't.  Do you want a spin off bug on that?

Filed bug 406416 - just a Hewitt typo from 2001.

> > Not sure what the right thing to do is.

I suspect the right thing is to narrow it down to a bug in the Windows native theming - the arrows aren't smashed against the text in Mac or Linux trunk, Tb or SeaMonkey, but I'm betting if you put a list-style-image on a button in Fx, you'll wind up with the same thing.
Assignee: mscott → nobody
Version: 2.0 → Trunk
I still see this
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.5) Gecko/20091215 Shredder/3.0.1pre
Severity: normal → trivial
Component: Mail Window Front End → Message Compose Window
QA Contact: front-end → message-compose
(Assignee)

Comment 6

5 years ago
Still valid on TB19.
So is it a problem with the icon in Windows theme or a toolkit bug of wrongly rendering the list-style-image (not enough padding)?

Comment 7

5 years ago
Created attachment 676126 [details]
old classic theme style

This gives you a rough approximation of what the Classic theme used to look like in the Mozilla suite. As you can see the arrow images contained some built-in padding (I believe they were basically 11x11 transparent squares with the arrow drawn in black).

At some point the arrow images were cropped to the minimum size, however presumably nobody bothered to check non-Firefox users...

Comment 8

5 years ago
(In reply to comment #7)
> At some point the arrow images were cropped to the minimum size
And the arrows themselves were reduced in size too it seems.
(Assignee)

Comment 9

5 years ago
Maybe it is just weird to apply list-style-image onto a button element. Can we rewrite it as <button><image/><label></button> ?
Flags: needinfo?(neil)
(Assignee)

Comment 10

5 years ago
Or even <button image=""> ?
(In reply to aceman from comment #9)
> Maybe it is just weird to apply list-style-image onto a button element.
It's an inheritable CSS property, so the image inherits it from the button. We do that with most of our images, although sometimes people forget and write less efficient CSS that targets the image directly instead.

> Can we rewrite it as <button><image/><label></button> ?
We could do, and that would let us set a class on the image to give it padding, while still inheriting the image from the button. (If you don't want to inherit the image from the button then you have to fiddle about getting the script to update the attribute on the image instead of the button.)

(I would suggest doing the MoreFewerButton while you're there but it turns out it's only used in hidden dialogs - you would have to manually insert a form control before you could open the dialog in question.)

(In reply to aceman from comment #10)
> Or even <button image=""> ?
That would make no sense at all.
Flags: needinfo?(neil)
(Assignee)

Comment 12

5 years ago
(In reply to neil@parkwaycc.co.uk from comment #11)
> (In reply to aceman from comment #10)
> > Or even <button image=""> ?
> That would make no sense at all.

Is <button image="..." label="..."> not rendered they way we want it here? I'll check it out.
Assignee: nobody → acelists
Setting the image through the attribute achieves the same result as setting it through CSS except that you fix the location of the image (setting it in CSS only fixes the location of the stylesheet, but themes expect that).
It could also be possible to add the old arrow icons in qute and point in CSS to them instead to global.
(Assignee)

Comment 15

5 years ago
It seems rewriting EdTableProps.xul to:
<button id="PreviousButton" accesskey="&cellSelectPrevious.accessKey;" oncommand="MoveSelection(0)" flex="1"><image/><label value="&cellSelectPrevious.label;"/></button>
<button id="NextButton" class="align-right"  accesskey="&cellSelectNext.accessKey;" oncommand="MoveSelection(1)" flex="1"><image/><label value="&cellSelectNext.label;"/></button>

seems to work without any changes to css.

Only that the accesskey is not working yet, nor on <label>. I'll need to find out where to put it.
(In reply to aceman from comment #15)
> It seems rewriting EdTableProps.xul to:
> <button id="PreviousButton" accesskey="&cellSelectPrevious.accessKey;"
> oncommand="MoveSelection(0)" flex="1"><image/><label
> value="&cellSelectPrevious.label;"/></button>
> <button id="NextButton" class="align-right" 
> accesskey="&cellSelectNext.accessKey;" oncommand="MoveSelection(1)"
> flex="1"><image/><label value="&cellSelectNext.label;"/></button>
> 
> seems to work without any changes to css.
> 
> Only that the accesskey is not working yet, nor on <label>. I'll need to
> find out where to put it.

[Wild guess] Try:
<button id="PreviousButton" oncommand="MoveSelection(0)" flex="1"><image/><label value="&cellSelectPrevious.label;" accesskey="&cellSelectPrevious.accessKey;" control="PreviousButton"/></button>
(Assignee)

Comment 17

5 years ago
Yes, found that out too.
This seems to work and render fine, on Win XP:

<button id="PreviousButton" oncommand="MoveSelection(0)" flex="1"><image/><label value="&cellSelectPrevious.label;" accesskey="&cellSelectPrevious.accessKey;" control="PreviousButton"/></button>
<button id="NextButton" class="align-right" oncommand="MoveSelection(1)" flex="1"><image/><label value="&cellSelectNext.label;" accesskey="&cellSelectNext.accessKey;" control="NextButton"/></button>

Even the changing to "row" changes the icons fine.
(Assignee)

Comment 18

5 years ago
Neil, would you be OK with this solution? The patch would consist only of these 2 lines. Or do I need to do anything else? You mentioned some MoreFewerButton but that is in a different file so I do not understand what to do with that and how it is related.
Flags: needinfo?(neil)
(In reply to aceman from comment #18)
> Neil, would you be OK with this solution? The patch would consist only of
> these 2 lines. Or do I need to do anything else? You mentioned some
> MoreFewerButton but that is in a different file so I do not understand what
> to do with that and how it is related.

Sure, we'll take a look at this solution, and then I can always look into porting the changes to work with the MoreFewerButton.
Flags: needinfo?(neil)
(Assignee)

Comment 20

5 years ago
Created attachment 676273 [details] [diff] [review]
patch
Attachment #676273 - Flags: review?(neil)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
Comment on attachment 676273 [details] [diff] [review]
patch

I guess this is about as good as we can do without replacement arrow images.
Attachment #676273 - Flags: review?(neil) → review+

Updated

5 years ago
Component: Message Compose Window → Composer
Keywords: checkin-needed
Product: Thunderbird → SeaMonkey
https://hg.mozilla.org/comm-central/rev/95500eb42b5f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.16
I believe this isn't specific to SeaMonkey, but affects Thunderbird as well in the compose window (the properties dialog is there)
Component: Composer → Composition
Product: SeaMonkey → MailNews Core
Target Milestone: seamonkey2.16 → ---
Mark Banner changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|Composer                    |Composition
            Product|SeaMonkey                   |MailNews Core

I admit I'm not sure what the best fit for editor/ui/dialogs/content is.
Target Milestone: --- → Thunderbird 19.0
I guess I should have done that as some sort of quote but you can't easily quote field changes in Bugzilla ;-)
You need to log in before you can comment on or make changes to this bug.