940286 - Australis bookmarks menu does not highlight on typing

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[B.J. Herbison]

Steps to reproduce:
1) Click the bookmarks menu button.
2) Type "M".
3) Press the right arrow key.

Expected result:
The "Mozilla Firefox" entry is highlighted in step 2.

Actual result:
No visual sign that Firefox has noticed the typing in step 2, but the menu does open in step 3.

Comment 1

[B.J. Herbison]

This behavior occurs if the item is a link or a bookmarks folder with no sub-folders. Bookmarks folders with sub-folders get highlighted by typing.

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Keyboard navigation of menus sets the [_moz-menuactive] attribute.

Comment 3

[Dão Gottwald [:dao]]

Using this selector for both toolbarbuttons and menuitems is kind of a mess...

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

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

Review of attachment 8363992 [details] [diff] [review]:
-----------------------------------------------------------------

It would be great to clean up the toolbarbutton/menuitem mess that we have with the menu panel and subviews, but I don't expect that to happen anytime soon.

Comment 5

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/fx-team/rev/0592b1488a8a

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0592b1488a8a

Comment 7

[Mike de Boer [:mikedeboer]]

This needs to be backed out, because `_moz_menuactive == "true"` on mouse hover. This causes the menu items on mouse hover to receive the ACTIVE button styling, which is darker than the HOVER styles.

In other words, this is not the right fix.

Comment 8

[Matt Brubeck (:mbrubeck)]

Attachment: follow-up patch

Good catch, thanks.  This patch (when applied on top of the first one) applies "hover" styling instead of "active" styling for [_moz-menuactive="true"] menuitems.

Comment 9

[Mike de Boer [:mikedeboer]]

Backed out: https://hg.mozilla.org/integration/fx-team/rev/3021d13c9cff

(pending review and checkin of updated patch)

Comment 10

[:Gijs (he/him)]

Comment on attachment 8366068 [details] [diff] [review]
follow-up patch

Review of attachment 8366068 [details] [diff] [review]:
-----------------------------------------------------------------

Stealing this review. This won't apply cleanly anymore now that it's been backed out, but the gist of this is sound. I trust your rebasing skills. r=me.

Comment 11

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/fx-team/rev/4b86247721b2

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4b86247721b2