Commit 943e158f authored by Tatsuhisa Yamaguchi's avatar Tatsuhisa Yamaguchi Committed by Commit Bot

Prevent focusing an invisible item in context menu by keyboard.

We have been using hidden=true attribute to hide some menu items.
We started to use display=none CSS attiribute for the same purpose,
so that it can change visibility of items by changing an attribute of
the parent node.
https://chromium-review.googlesource.com/c/566768/15/ui/file_manager/file_manager/foreground/css/file_manager.css
The logic for keyboard focus should also skip items hidden by that way.

Bug: 748504
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ia4c6cffcc9ac5e64c64da7644622ebf2949727e0
Reviewed-on: https://chromium-review.googlesource.com/597386Reviewed-by: default avatarMichael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491926}
parent eb34528a
......@@ -184,6 +184,22 @@ cr.define('cr.ui', function() {
return this.menuItems.length;
},
/**
* Returns whether the given menu item is visible.
* @param {!cr.ui.MenuItem} menuItem
* @return {boolean}
* @private
*/
isItemVisible_: function(menuItem) {
if (menuItem.hidden)
return false;
if (!!menuItem.offsetParent)
return true;
// A "position: fixed" element won't have an offsetParent, so we have to
// do the full style computation.
return window.getComputedStyle(menuItem).display != 'none';
},
/**
* Returns if the menu has any visible item.
* @return {boolean} True if the menu has visible item. Otherwise, false.
......@@ -191,7 +207,7 @@ cr.define('cr.ui', function() {
hasVisibleItems: function() {
var menuItems = this.menuItems; // Cache.
for (var i = 0, menuItem; menuItem = menuItems[i]; i++) {
if (!menuItem.isSeparator() && !menuItem.hidden)
if (!menuItem.isSeparator() && this.isItemVisible_(menuItem))
return true;
}
return false;
......@@ -207,7 +223,7 @@ cr.define('cr.ui', function() {
var item = this.selectedItem;
var self = this;
function selectNextAvailable(m) {
var selectNextAvailable = function(m) {
var menuItems = self.menuItems;
var len = menuItems.length;
if (!len) {
......@@ -234,12 +250,13 @@ cr.define('cr.ui', function() {
break;
item = menuItems[i];
if (item && !item.isSeparator() && !item.hidden && !item.disabled)
if (item && !item.isSeparator() && !item.disabled &&
this.isItemVisible_(item))
break;
}
if (item && !item.disabled)
self.selectedIndex = i;
}
}.bind(this);
switch (e.key) {
case 'ArrowDown':
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment