Skip to content

Commit 2280876

Browse files
wolfibDevtools-frontend LUCI CQ
authored andcommitted
Reland "Context menu: fix event handling on close"
This is a reland of commit dcc0329 The original CL caused e2e test failures on Mac bots. This reland aims to fix this by awaiting the context menus to be fully closed before moving on. Original change's description: > Context menu: fix event handling on close > > On Windows, closing a context menu by clicking on the button which > opened it, would immediately re-open the menu. A fix was added in > https://crrev.com/c/5592569, which relies on `requestAnimationFrame`. > rAF does not trigger when when the window is not visible, which can > happen for the device toolbar (rendered in the main browser window) > when DevTools is detached and hidden behind another window. > > This CL replaces `requestAnimationFrame` with `setTimeout`. > > Bug: 377161958, 339560549 > Change-Id: Id6af436c41d14a6e16160faff4e160e40bbe4122 > Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6004964 > Auto-Submit: Wolfgang Beyer <[email protected]> > Reviewed-by: Benedikt Meurer <[email protected]> > Commit-Queue: Benedikt Meurer <[email protected]> Bug: 377161958, 339560549 Change-Id: I3bef672cbfa201f7b53aa3003a709789394a8b0a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6017683 Auto-Submit: Wolfgang Beyer <[email protected]> Reviewed-by: Benedikt Meurer <[email protected]> Commit-Queue: Benedikt Meurer <[email protected]>
1 parent 6557eac commit 2280876

File tree

5 files changed

+37
-6
lines changed

5 files changed

+37
-6
lines changed

front_end/ui/legacy/Toolbar.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,9 +1028,9 @@ export class ToolbarMenuButton extends ToolbarCombobox {
10281028
useSoftMenu: this.useSoftMenu,
10291029
x: this.element.getBoundingClientRect().left,
10301030
y: this.element.getBoundingClientRect().top + this.element.offsetHeight,
1031-
// Without rAF, pointer events will be un-ignored too early, and a single click causes the
1032-
// context menu to be closed and immediately re-opened on Windows (https://crbug.com/339560549).
1033-
onSoftMenuClosed: () => requestAnimationFrame(() => this.element.removeAttribute('aria-expanded')),
1031+
// Without adding a delay, pointer events will be un-ignored too early, and a single click causes
1032+
// the context menu to be closed and immediately re-opened on Windows (https://crbug.com/339560549).
1033+
onSoftMenuClosed: () => setTimeout(() => this.element.removeAttribute('aria-expanded'), 50),
10341034
});
10351035
this.contextMenuHandler(contextMenu);
10361036
this.element.setAttribute('aria-expanded', 'true');

test/e2e/emulation/responsive_test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import {
77
pressKey,
88
waitFor,
99
} from '../../shared/helper.js';
10-
1110
import {
1211
clickWidthInput,
1312
clickZoomDropDown,
1413
getZoom,
1514
openDeviceToolbar,
1615
reloadDockableFrontEnd,
1716
toggleAutoAdjustZoom,
17+
waitForZoomDropDownNotExpanded,
1818
} from '../helpers/emulation-helpers.js';
1919

2020
describe('Custom devices', () => {
@@ -24,10 +24,11 @@ describe('Custom devices', () => {
2424
await openDeviceToolbar();
2525
});
2626

27-
it('can preserved zoom', async () => {
27+
it('can preserve zoom', async () => {
2828
await clickZoomDropDown();
2929
await pressKey('ArrowDown');
3030
await pressKey('Enter');
31+
await waitForZoomDropDownNotExpanded();
3132
assert.strictEqual(await getZoom(), '50%');
3233

3334
await clickWidthInput();
@@ -37,6 +38,7 @@ describe('Custom devices', () => {
3738
await clickZoomDropDown();
3839
await pressKey('ArrowDown');
3940
await pressKey('Enter');
41+
await waitForZoomDropDownNotExpanded();
4042
assert.strictEqual(await getZoom(), '50%');
4143

4244
await toggleAutoAdjustZoom();

test/e2e/helpers/emulation-helpers.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
getBrowserAndPages,
1111
goToResource,
1212
waitFor,
13+
waitForFunction,
1314
} from '../../shared/helper.js';
1415

1516
import {
@@ -100,32 +101,51 @@ export const selectToggleButton = async () => {
100101
export const selectEdit = async () => {
101102
await clickDevicesDropDown();
102103
await click(EDIT_MENU_ITEM_SELECTOR);
104+
await waitForNotExpanded(DEVICE_LIST_DROPDOWN_SELECTOR);
103105
};
104106

105107
export const selectDevice = async (name: string) => {
106108
await clickDevicesDropDown();
107109
await click(`[aria-label*="${name}, unchecked"]`);
110+
await waitForNotExpanded(DEVICE_LIST_DROPDOWN_SELECTOR);
108111
};
109112

110113
export const selectTestDevice = async () => {
111114
await clickDevicesDropDown();
112115
await click(TEST_DEVICE_MENU_ITEM_SELECTOR);
116+
await waitForNotExpanded(DEVICE_LIST_DROPDOWN_SELECTOR);
113117
};
114118

115119
// Test if span button works when emulating a dual screen device.
116120
export const selectDualScreen = async () => {
117121
await clickDevicesDropDown();
118122
await click(SURFACE_DUO_MENU_ITEM_SELECTOR);
123+
await waitForNotExpanded(DEVICE_LIST_DROPDOWN_SELECTOR);
119124
};
120125

121126
export const selectFoldableDevice = async () => {
122127
await clickDevicesDropDown();
123128
await click(FOLDABLE_DEVICE_MENU_ITEM_SELECTOR);
129+
await waitForNotExpanded(DEVICE_LIST_DROPDOWN_SELECTOR);
130+
};
131+
132+
const waitForNotExpanded = async (selector: string) => {
133+
const toolbar = await waitFor(DEVICE_TOOLBAR_SELECTOR);
134+
const dropdown = await waitFor(selector, toolbar);
135+
await waitForFunction(async () => {
136+
const expanded = await dropdown.evaluate(el => el.getAttribute('aria-expanded'));
137+
return expanded === null;
138+
});
139+
};
140+
141+
export const waitForZoomDropDownNotExpanded = async () => {
142+
await waitForNotExpanded(ZOOM_LIST_DROPDOWN_SELECTOR);
124143
};
125144

126145
export const clickDevicePosture = async (name: string) => {
127146
await clickDevicePostureDropDown();
128147
await click(`[aria-label*="${name}, unchecked"]`);
148+
await waitForNotExpanded(DEVICE_POSTURE_DROPDOWN_SELECTOR);
129149
};
130150

131151
export const getDevicePostureDropDown = async () => {
@@ -155,6 +175,7 @@ export const getZoom = async () => {
155175
export const toggleAutoAdjustZoom = async () => {
156176
await clickZoomDropDown();
157177
await click(AUTO_AUTO_ADJUST_ZOOM_SELECTOR);
178+
await waitForZoomDropDownNotExpanded();
158179
};
159180

160181
const IPAD_MENU_ITEM_SELECTOR = '[aria-label*="iPad"]';
@@ -163,4 +184,5 @@ const IPAD_MENU_ITEM_SELECTOR = '[aria-label*="iPad"]';
163184
export const selectNonDualScreenDevice = async () => {
164185
await clickDevicesDropDown();
165186
await click(IPAD_MENU_ITEM_SELECTOR);
187+
await waitForNotExpanded(DEVICE_LIST_DROPDOWN_SELECTOR);
166188
};

test/e2e/helpers/settings-helpers.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ export const openPanelViaMoreTools = async (panelTitle: string) => {
3131
// Click the desired menu item
3232
await click(`aria/${panelTitle}[role="menuitem"]`);
3333

34+
// Wait for the triple dot menu to be collapsed.
35+
const button = await waitForAria('Customize and control DevTools');
36+
await waitForFunction(async () => {
37+
const expanded = await button.evaluate(el => el.getAttribute('aria-expanded'));
38+
return expanded === null;
39+
});
40+
3441
// Wait for the corresponding panel to appear.
3542
await waitForAria(`${panelTitle} panel[role="tabpanel"]`);
3643
};

test/e2e/recorder/replay_test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,7 @@ describe('Recorder', function() {
658658
);
659659
});
660660

661-
it('should be able to navigate to a prerendered page', async () => {
661+
it('should be able to navigate to a prerendered page', async () => {
662662
await setupRecorderWithScriptAndReplay({
663663
title: 'Test Recording',
664664
steps: [

0 commit comments

Comments
 (0)