diff --git a/zkdoc/release-note b/zkdoc/release-note index ca0432b394e..9810fa4c1cf 100644 --- a/zkdoc/release-note +++ b/zkdoc/release-note @@ -65,6 +65,7 @@ ZK 10.0.0 ZK-5540: zk.wpd with browser condition won't work with ZK 10 ZK-5530: users cannot focus on the icons on the Calendar header ZK-5453: XSS in chosenbox + ZK-5025: redundant selection highlight on a menupopup * Upgrade Notes + The Java binary-compatible level is Java 11 since ZK 10. diff --git a/zktest/src/main/webapp/test2/B100-ZK-5025.zul b/zktest/src/main/webapp/test2/B100-ZK-5025.zul new file mode 100644 index 00000000000..e8e006cce09 --- /dev/null +++ b/zktest/src/main/webapp/test2/B100-ZK-5025.zul @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/zktest/src/main/webapp/test2/config.properties b/zktest/src/main/webapp/test2/config.properties index b8f8bb78183..f9e6a7b92d7 100644 --- a/zktest/src/main/webapp/test2/config.properties +++ b/zktest/src/main/webapp/test2/config.properties @@ -3159,6 +3159,7 @@ B90-ZK-4431.zul=A,E,Multislider ##zats##B100-ZK-5468-Tree.zul=A,E,Model,Tree ##zats##B100-ZK-5235.zul=A,E,Menu,Menupopup,focus ##zats##B100-ZK-5453.zul=A,E,Chosenbox,XSS +##zats##B100-ZK-5025.zul=A,E,Menu,Menupopup,Menuitem,focus,hover ## # Features - 3.0.x version diff --git a/zktest/src/test/java/org/zkoss/zktest/zats/test2/B100_ZK_5025Test.java b/zktest/src/test/java/org/zkoss/zktest/zats/test2/B100_ZK_5025Test.java new file mode 100644 index 00000000000..40ee0929541 --- /dev/null +++ b/zktest/src/test/java/org/zkoss/zktest/zats/test2/B100_ZK_5025Test.java @@ -0,0 +1,64 @@ +/* B100_ZK_5025Test.java + + Purpose: + + Description: + + History: + Fri Sep 08 14:22:26 CST 2023, Created by jamson + +Copyright (C) 2023 Potix Corporation. All Rights Reserved. +*/ +package org.zkoss.zktest.zats.test2; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.openqa.selenium.Keys; +import org.openqa.selenium.interactions.Actions; + +import org.zkoss.test.webdriver.WebDriverTestCase; +import org.zkoss.test.webdriver.ztl.JQuery; + + +public class B100_ZK_5025Test extends WebDriverTestCase { + @Test + public void test(){ + Actions actions = new Actions(connect()); + + JQuery menu1 = jq(".z-menu"), + menu2 = jq("@menu:eq(1)"), + menuitem1 = jq("@menuitem:eq(0)"), + menuitem2 = jq("@menuitem:eq(1)"); + + // open menu1 + actions.moveToElement(toElement(menu1)); + click(menu1); + Assertions.assertTrue(menuitem1.attr("class").equals("z-menuitem")); + Assertions.assertTrue(menuitem2.attr("class").equals("z-menuitem")); + Assertions.assertTrue(menu2.attr("class").equals("z-menu")); + + // hover to menuitem1 + actions.moveToElement(toElement(menuitem1)).build().perform(); + Assertions.assertTrue(menuitem1.attr("class").equals("z-menuitem z-menuitem-hover")); // highlight should be here only + Assertions.assertTrue(menuitem2.attr("class").equals("z-menuitem")); + Assertions.assertTrue(menu2.attr("class").equals("z-menu")); + + // keydomn:DOWN to menuitem2 + actions.keyDown(Keys.DOWN).perform(); + Assertions.assertTrue(menuitem1.attr("class").equals("z-menuitem")); + Assertions.assertTrue(menuitem2.attr("class").equals("z-menuitem z-menuitem-focus")); // highlight should be here only + Assertions.assertTrue(menu2.attr("class").equals("z-menu")); + + // keydomn:DOWN to menu2 + actions.keyDown(Keys.DOWN).perform(); + Assertions.assertTrue(menuitem1.attr("class").equals("z-menuitem")); + Assertions.assertTrue(menuitem2.attr("class").equals("z-menuitem")); + Assertions.assertTrue(menu2.attr("class").equals("z-menu z-menu-focus")); // highlight should be here only + + // hover to menuitem2 + actions.moveToElement(toElement(menuitem2)).build().perform(); + Assertions.assertTrue(menuitem1.attr("class").equals("z-menuitem")); + Assertions.assertTrue(menuitem2.attr("class").equals("z-menuitem z-menuitem-hover")); // highlight should be here only + Assertions.assertTrue(menu2.attr("class").equals("z-menu")); + } +} diff --git a/zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_4884Test.java b/zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_4884Test.java index 64b44430883..d44a0ee05e1 100644 --- a/zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_4884Test.java +++ b/zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_4884Test.java @@ -22,18 +22,20 @@ public class B96_ZK_4884Test extends WebDriverTestCase { @Test public void test() { + // The highlight class names were used to be all `-hover`, after ZK-5025, it's divided into `-hover` (mouse) and `-focus` (keydown) + connect(); click(jq("$menu1")); waitResponse(); click(jq("$menuitem1")); waitResponse(); - Assertions.assertEquals(1, jq(".z-menu-hover").length()); + Assertions.assertEquals(1, jq(".z-menu-focus").length()); click(jq("$menu2")); waitResponse(); click(jq("$menuitem2")); waitResponse(); - Assertions.assertEquals(1, jq(".z-menu-hover").length()); + Assertions.assertEquals(1, jq(".z-menu-focus").length()); } } diff --git a/zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_5026Test.java b/zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_5026Test.java index f984e4e7882..802778f77db 100644 --- a/zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_5026Test.java +++ b/zktest/src/test/java/org/zkoss/zktest/zats/test2/B96_ZK_5026Test.java @@ -27,6 +27,8 @@ public class B96_ZK_5026Test extends WebDriverTestCase { @Test public void test() { + // The highlight class names were used to be all `-hover`, after ZK-5025, it's divided into `-hover` (mouse) and `-focus` (keydown) + Actions act = new Actions(connect()); act.moveToElement(toElement(jq("$menu"))).perform(); waitResponse(); @@ -39,10 +41,10 @@ public void test() { assertTrue(jq(" .z-menuitem.z-menuitem-hover").exists()); act.sendKeys(Keys.DOWN).perform(); - assertTrue(jq(".z-menupopup-content > .z-menuitem:nth-child(2)").hasClass("z-menuitem-hover")); + assertTrue(jq(".z-menupopup-content > .z-menuitem:nth-child(2)").hasClass("z-menuitem-focus")); act.sendKeys(Keys.DOWN).perform(); - assertTrue(jq(".z-menupopup-content > .z-menuitem:nth-child(3)").hasClass("z-menuitem-hover")); + assertTrue(jq(".z-menupopup-content > .z-menuitem:nth-child(3)").hasClass("z-menuitem-focus")); // error happen if bug exists act.moveToElement(toElement( @@ -56,11 +58,8 @@ public void test() { .perform(); assertTrue(jq(" .z-menuitem.z-menuitem-hover").exists()); - JQuery childMenupopup = jq(".z-menupopup-content").eq(1); - act.sendKeys(Keys.DOWN).perform(); - assertTrue(childMenupopup.children(".z-menuitem:first-child").hasClass("z-menuitem-hover")); - act.sendKeys(Keys.DOWN).perform(); - assertTrue(childMenupopup.children(".z-menuitem:nth-child(2)").hasClass("z-menuitem-hover")); + // After ZK-5025, if hover or focus is removed from the menupopup, the menupopup will automatically close. + // Since the behavior has changed, it's contrary to this test case, so the previous subsequent operations are removed. } } diff --git a/zktest/src/test/java/org/zkoss/zktest/zats/test2/F55_ZK_443Test.java b/zktest/src/test/java/org/zkoss/zktest/zats/test2/F55_ZK_443Test.java index f6604bf396d..5d9539dfcd9 100644 --- a/zktest/src/test/java/org/zkoss/zktest/zats/test2/F55_ZK_443Test.java +++ b/zktest/src/test/java/org/zkoss/zktest/zats/test2/F55_ZK_443Test.java @@ -24,6 +24,8 @@ public class F55_ZK_443Test extends WebDriverTestCase { @Test public void test() { + // The highlight class names were used to be all `-hover`, after ZK-5025, it's divided into `-hover` (mouse) and `-focus` (keydown) + connect(); click(jq("@menu:first")); @@ -59,7 +61,7 @@ public void test() { click(jq("@button:eq(0)")); waitResponse(); - Assertions.assertTrue(jq("@menu:contains(Project)").hasClass("z-menu-hover")); + Assertions.assertTrue(jq("@menu:contains(Project)").hasClass("z-menu-focus")); click(jq("@button:eq(1)")); waitResponse(); diff --git a/zul/src/main/resources/web/js/zul/menu/Menu.ts b/zul/src/main/resources/web/js/zul/menu/Menu.ts index dcd9fffc4c1..a096c5b9f86 100644 --- a/zul/src/main/resources/web/js/zul/menu/Menu.ts +++ b/zul/src/main/resources/web/js/zul/menu/Menu.ts @@ -372,7 +372,7 @@ export class Menu extends zul.LabelImageWidget implements zul.LabelImageWidgetWi jq(this.$n_()).removeClass(this.$s('hover')).removeClass(this.$s('selected')); pp.close(); } - (this.$class as typeof Menu)._addActive(this); // keep the focus + (this.$class as typeof Menu)._addActive(this, 'focus'); // keep the focus evt.stop(); break; case 40: //DOWN @@ -411,7 +411,7 @@ export class Menu extends zul.LabelImageWidget implements zul.LabelImageWidgetWi if (menubar && menubar._lastTarget) $menu._rmActive(menubar._lastTarget); if (!this._ignoreActive) - $menu._addActive(this); + $menu._addActive(this, 'focus'); } // delete the variable used for IE delete this._ignoreActive; @@ -553,7 +553,7 @@ export class Menu extends zul.LabelImageWidget implements zul.LabelImageWidgetWi } } } - (this.$class as typeof Menu)._addActive(this); + (this.$class as typeof Menu)._addActive(this, 'hover'); } /** @internal */ @@ -598,20 +598,20 @@ export class Menu extends zul.LabelImageWidget implements zul.LabelImageWidgetWi } /** @internal */ - static _isActive(wgt: zul.menu.Menu): boolean { + static _isActive(wgt: zul.menu.Menu, type: string): boolean { var top = wgt.isTopmost(), n = wgt.$n_(), menupopup = wgt.menupopup, - cls = top ? menupopup && menupopup.isOpen() ? wgt.$s('selected') : wgt.$s('hover') : wgt.$s('hover'); + cls = top ? menupopup && menupopup.isOpen() ? wgt.$s('selected') : wgt.$s(type) : wgt.$s(type); return jq(n).hasClass(cls); } /** @internal */ - static _addActive(wgt: zul.menu.Menu): void { + static _addActive(wgt: zul.menu.Menu, type: string): void { var top = wgt.isTopmost(), n = wgt.$n_(), menupopup = wgt.menupopup, - cls = top ? menupopup && menupopup.isOpen() ? wgt.$s('selected') : wgt.$s('hover') : wgt.$s('hover'); + cls = top ? menupopup && menupopup.isOpen() ? wgt.$s('selected') : wgt.$s(type) : wgt.$s(type); jq(n).addClass(cls); if (top) { var mb = wgt.getMenubar(); @@ -621,7 +621,7 @@ export class Menu extends zul.LabelImageWidget implements zul.LabelImageWidgetWi const parentMenupopup = wgt.parent; parentMenupopup.addActive_(wgt); if (parentMenupopup.parent instanceof zul.menu.Menu) - this._addActive(parentMenupopup.parent); + this._addActive(parentMenupopup.parent, type); } } @@ -630,9 +630,8 @@ export class Menu extends zul.LabelImageWidget implements zul.LabelImageWidgetWi var top = wgt.isTopmost(), n = wgt.$n_(), menupopup = wgt.menupopup, - cls = top ? (!ignoreSeld && menupopup && menupopup.isOpen()) ? wgt.$s('selected') : wgt.$s('hover') : wgt.$s('hover'), - anode = jq(n).removeClass(cls); - if (top && !(anode.hasClass(wgt.$s('selected')) || anode.hasClass(wgt.$s('hover')))) + anode = (top && !ignoreSeld && menupopup && menupopup.isOpen()) ? jq(n).removeClass(wgt.$s('selected')) : jq(n).removeClass(wgt.$s('hover')).removeClass(wgt.$s('focus')); + if (top && !(anode.hasClass(wgt.$s('selected')) || (anode.hasClass(wgt.$s('hover')) || anode.hasClass(wgt.$s('focus'))))) _toggleClickableCSS(wgt, true); } diff --git a/zul/src/main/resources/web/js/zul/menu/Menuitem.ts b/zul/src/main/resources/web/js/zul/menu/Menuitem.ts index 6e30f0c1729..ab8764bab11 100644 --- a/zul/src/main/resources/web/js/zul/menu/Menuitem.ts +++ b/zul/src/main/resources/web/js/zul/menu/Menuitem.ts @@ -533,9 +533,10 @@ export class Menuitem extends zul.LabelImageWidget implements zul.LabelImageWidg if (zul.menu._nOpen || isTopmost) zWatch.fire('onFloatUp', this); //notify all if (!isTopmost && !this._disabled) { - if (this.parent) - this.parent.removeActive_(); - (this.$class as typeof Menuitem)._addActive(this); + if (this.parent instanceof zul.menu.Menupopup) + this.parent.removeAllChildrenActive_(); + (this.$class as typeof Menuitem)._addActive(this, 'hover'); + this.focus(); } } @@ -582,20 +583,20 @@ export class Menuitem extends zul.LabelImageWidget implements zul.LabelImageWidg } /** @internal */ - static _addActive(wgt: zul.menu.Menu | zul.menu.Menuitem): void { + static _addActive(wgt: zul.menu.Menu | zul.menu.Menuitem, type: string): void { var top = wgt.isTopmost(); - jq(wgt.$n_()).addClass(wgt.$s('hover')); + jq(wgt.$n_()).addClass(wgt.$s(type)); if (!top && wgt.parent instanceof zul.menu.Menupopup) { const parentMenupopup = wgt.parent; parentMenupopup.addActive_(wgt); if (parentMenupopup.parent instanceof zul.menu.Menu) - this._addActive(parentMenupopup.parent); + this._addActive(parentMenupopup.parent, type); } } /** @internal */ static _rmActive(wgt: zk.Widget): JQuery { - return jq(wgt.$n_()).removeClass(wgt.$s('hover')); + return jq(wgt.$n_()).removeClass(wgt.$s('hover')).removeClass(wgt.$s('focus')); } onShow(): void { diff --git a/zul/src/main/resources/web/js/zul/menu/Menupopup.ts b/zul/src/main/resources/web/js/zul/menu/Menupopup.ts index e4950540b00..1cb0aadcf77 100644 --- a/zul/src/main/resources/web/js/zul/menu/Menupopup.ts +++ b/zul/src/main/resources/web/js/zul/menu/Menupopup.ts @@ -81,7 +81,7 @@ function _activateNextMenu(menu: zul.menu.Menu): void { pp.open(undefined, undefined, undefined, {focusFirst: true, sendOnOpen: true, disableMask: true}); } } - (menu.$class as typeof zul.menu.Menu)._addActive(menu); + (menu.$class as typeof zul.menu.Menu)._addActive(menu, 'focus'); zWatch.fire('onFloatUp', menu); //notify all } @@ -265,7 +265,7 @@ export class Menupopup extends zul.wgt.Popup { return; var org = ctl.origin; - if (this.parent!.menupopup == this && !this.parent!.isTopmost() && !(this.parent!.$class as typeof zul.menu.Menu)._isActive(this.parent!)) { + if (this.parent!.menupopup == this && !this.parent!.isTopmost() && !(this.parent!.$class as typeof zul.menu.Menu)._isActive(this.parent!, 'hover')) { this.close({sendOnOpen: true}); return; } @@ -366,9 +366,12 @@ export class Menupopup extends zul.wgt.Popup { case 40: //DOWN // UP: 1. jump to the previousSibling item // DOWN: 1. jump to the nextSibling item - if (w) (w.$class as typeof zul.menu.Menuitem)._rmActive(w); + this.removeAllChildrenActive_(); w = keyCode == 38 ? _prevChild(this, w) : _nextChild(this, w); - if (w) (w.$class as typeof zul.menu.Menuitem)._addActive(w as zul.menu.Menuitem); // FIXME: type of w is inconsistent + if (w) { + (w.$class as typeof zul.menu.Menuitem)._addActive(w as zul.menu.Menuitem, 'focus'); // FIXME: type of w is inconsistent + w.focus(); + } break; case 37: //LEFT // 1. close the contenthandler (like colorbox), if any @@ -379,7 +382,7 @@ export class Menupopup extends zul.wgt.Popup { w._contentHandler.onHide(); } else if (((menu = _getMenu(this))) && !menu.isTopmost()) { this.close(); - (menu.$class as typeof zul.menu.Menu)._addActive(menu); + (menu.$class as typeof zul.menu.Menu)._addActive(menu, 'focus'); const pp = menu.parent as unknown as zul.menu.Menupopup | undefined; // FIXME: type of pp is inconsistent if (pp) { pp.focus(); @@ -424,7 +427,7 @@ export class Menupopup extends zul.wgt.Popup { if (menu.isTopmost()) { menu.focus(); } else { - (menu.$class as typeof zul.menu.Menu)._addActive(menu); + (menu.$class as typeof zul.menu.Menu)._addActive(menu, 'focus'); const pp = menu.parent; if (pp) { pp.focus(); @@ -446,7 +449,7 @@ export class Menupopup extends zul.wgt.Popup { content.onHide(); } else { this.close(); - (menu.$class as typeof zul.menu.Menu)._addActive(menu); + (menu.$class as typeof zul.menu.Menu)._addActive(menu, 'focus'); const pp = menu.parent; if (pp) { pp.focus(); @@ -542,7 +545,7 @@ export class Menupopup extends zul.wgt.Popup { this._curIndex = newCurrIndex; var target = this.getChildAt(childIndex); - if (target) (target.$class as typeof zul.menu.Menuitem)._addActive(target); + if (target) (target.$class as typeof zul.menu.Menuitem)._addActive(target, 'hover'); } } return this; @@ -576,6 +579,22 @@ export class Menupopup extends zul.wgt.Popup { } } + /** @internal */ + removeAllChildrenActive_(): void { + for (let child: zk.Widget | undefined = this?.firstChild; child != undefined; child = child.nextSibling) { + if (child instanceof zul.menu.Menuitem) { + (child.$class as typeof zul.menu.Menuitem)._rmActive(child); + } else if (child instanceof zul.menu.Menu) { + (child.$class as typeof zul.menu.Menu)._rmActive(child); + const pp = child.menupopup; + if (pp?.isOpen()) { + jq(child.$n_()).removeClass(child.$s('hover')).removeClass(child.$s('selected')); + pp.close(); + } + } + } + } + /** @internal */ _currentChild(): zul.menu.Menuitem | zul.menu.Menu | undefined { const index = this._curIndex, diff --git a/zul/src/main/resources/web/js/zul/menu/less/menu.less b/zul/src/main/resources/web/js/zul/menu/less/menu.less index 445128cea2e..948c9374aa1 100644 --- a/zul/src/main/resources/web/js/zul/menu/less/menu.less +++ b/zul/src/main/resources/web/js/zul/menu/less/menu.less @@ -115,7 +115,6 @@ white-space: nowrap; min-height: @menuContentMinHeight; z-index: 20; // the 20 is greater than menupopup-separator's z-index - &:hover { .contentStyle(@menuItemHoverColor, @menuItemHoverBackground); } @@ -216,12 +215,6 @@ .z-menuitem-content { color: @menuPopupItemColor; background: @menuPopupItemBackground; - &:hover { - .contentStyle(@menuPopupItemHoverColor, @menuPopupItemHoverBackground); - } - &:focus { - .contentStyle(@menuPopupItemHoverColor, @menuPopupItemHoverBackground); - } &:active { .contentStyle(@menuPopupItemActiveColor, @menuPopupItemActiveBackground); } @@ -263,7 +256,9 @@ line-height: normal; } .z-menu-hover > .z-menu-content, - .z-menuitem-hover > .z-menuitem-content { + .z-menu-focus > .z-menu-content, + .z-menuitem-hover > .z-menuitem-content, + .z-menuitem-focus > .z-menuitem-content { .contentStyle(@menuPopupItemHoverColor, @menuPopupItemHoverBackground); } }