534166 - Disabled arrow buttons in menus should be hidden

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[Markus Stange [:mstange]]

Attachment: v1

This patch is Mac-only, but I think Windows natively does the same. I haven't tested Linux.

Comment 1

[Dão Gottwald [:dao]]

Comment on attachment 417075 [details] [diff] [review]
v1

This doesn't seem to take horizontal scrollboxes into account.

> autorepeatbutton {
>+  -moz-appearance: menuitem;

I don't think this is appropriate. It seems that mozilla-central uses vertical arrowscrollboxes only in menus, but it's still a general-purpose binding.

Comment 2

[Markus Stange [:mstange]]

Attachment: v2

Comment 3

[Markus Stange [:mstange]]

Attachment: v3

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 419895 [details] [diff] [review]
v3

>+/* autorepeatbuttons in menus */

I'm afraid this belongs in menu.css, although this would currently spread the 14px across multiple files, which is ugly.

>+autorepeatbutton:not([orient="horizontal"]) {
>+  height: 14px;
>+}

Could the selector be menupopup > .popup-internal-box > .autorepeatbutton instead?

Comment 5

[Dão Gottwald [:dao]]

> >+autorepeatbutton:not([orient="horizontal"]) {
> >+  height: 14px;
> >+}
> 
> Could the selector be menupopup > .popup-internal-box > .autorepeatbutton
> instead?

Or, in order to get it to work, menupopup > .popup-internal-box > autorepeatbutton

Comment 6

[Markus Stange [:mstange]]

(In reply to comment #4)
> (From update of attachment 419895 [details] [diff] [review])
> >+/* autorepeatbuttons in menus */
> 
> I'm afraid this belongs in menu.css,

menu.css is only applied to menuitems and menuseparators, but not to the menupopup itself. So I can't target autorepeatbuttons from menu.css.

Comment 7

[Dão Gottwald [:dao]]

Then global.css maybe. I think scoped stylesheets shouldn't depend on stuff outside of their scope.

Comment 8

[Markus Stange [:mstange]]

Attachment: v4

OK, global.css.
I changed the selectors to only ".popup-internal-box > autorepeatbutton" (without "menupopup > ") because I think that's specific enough and because the other selector doesn't work in the bookmarks bar (there's an extra hbox in those menupopups).
I also noticed that the right size is 15px, not 14px ;)

Comment 9

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/5145176f8efd