Skip to content

Commit

Permalink
Revert disabling device emulation for chrome:// pages
Browse files Browse the repository at this point in the history
The original change to disable device emulation for chrome:// pages
introduces a race condition that results in inconsistent behavior.

When the user navigates from a chrome:// page to a non chrome:// page,
the navigation event causese the emulation to be enabled. However,
the page could already have been partially loaded when the emulation
setting takes effect. Depending on when this happens, the emulated
result could look different.

Since this approach is fundamentally unsound, let's revert this change
and instead accept that we'll have issue reports of chrome:// pages
that are not compatible with device emulation.

Fixed: 388411685
Change-Id: I690b5ed89c20602c46e590f89367dc5f775fd8ac
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6298051
Reviewed-by: Philip Pfaffe <[email protected]>
Auto-Submit: Yang Guo <[email protected]>
Commit-Queue: Philip Pfaffe <[email protected]>
  • Loading branch information
hashseed authored and Devtools-frontend LUCI CQ committed Feb 24, 2025
1 parent c4b6caa commit a2bcd79
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 66 deletions.
13 changes: 2 additions & 11 deletions front_end/panels/emulation/DeviceModeWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import * as Common from '../../core/common/common.js';
import type * as Common from '../../core/common/common.js';
import * as Root from '../../core/root/root.js';
import * as SDK from '../../core/sdk/sdk.js';
import type * as Protocol from '../../generated/protocol.js';
Expand Down Expand Up @@ -32,8 +32,6 @@ export class DeviceModeWrapper extends UI.Widget.VBox {
SDK.TargetManager.TargetManager.instance().addModelListener(
SDK.OverlayModel.OverlayModel, SDK.OverlayModel.Events.SCREENSHOT_REQUESTED,
this.screenshotRequestedFromOverlay, this);
SDK.TargetManager.TargetManager.instance().addEventListener(
SDK.TargetManager.Events.INSPECTED_URL_CHANGED, () => this.update(), this);
this.update(true);
}

Expand Down Expand Up @@ -85,14 +83,7 @@ export class DeviceModeWrapper extends UI.Widget.VBox {
update(force?: boolean): void {
this.toggleDeviceModeAction.setToggled(this.showDeviceModeSetting.get());

// Only allow device mode for non chrome:// pages.
function allowDeviceMode(): boolean {
const target = SDK.TargetManager.TargetManager.instance().primaryPageTarget();
const url = target?.inspectedURL();
return url ? !Common.ParsedURL.schemeIs(url, 'chrome:') : false;
}

const shouldShow = this.showDeviceModeSetting.get() && allowDeviceMode();
const shouldShow = this.showDeviceModeSetting.get();
if (!force && shouldShow === this.deviceModeView?.isShowing()) {
return;
}
Expand Down
1 change: 0 additions & 1 deletion test/e2e/emulation/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import("../../../scripts/build/typescript/typescript.gni")
node_ts_library("emulation") {
sources = [
"custom-devices_test.ts",
"disable-device-mode_test.ts",
"dual-screen_test.ts",
"foldable-device_test.ts",
"media-query-inspector_test.ts",
Expand Down
45 changes: 0 additions & 45 deletions test/e2e/emulation/disable-device-mode_test.ts

This file was deleted.

9 changes: 0 additions & 9 deletions test/e2e/helpers/emulation-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,6 @@ export const reloadDockableFrontEnd = async () => {
await reloadDevTools({canDock: true});
};

export const deviceModeIsToggled = async () => {
const deviceToolbarToggler = await waitFor(DEVICE_TOOLBAR_TOGGLER_SELECTOR);
const pressed = await deviceToolbarToggler.evaluate(element => {
const button = element.shadowRoot?.querySelector('.primary-toggle') as HTMLButtonElement;
return button.getAttribute('aria-pressed');
});
return pressed === 'true';
};

export const deviceModeIsEnabled = async () => {
// Check the userAgent string to see whether emulation is really enabled.
const {target} = getBrowserAndPages();
Expand Down

0 comments on commit a2bcd79

Please sign in to comment.