fix(devtools): catch firefox non-top level frame error case when using utilities APIs (#60430)

Previously the `frameUrl` option in `chrome.devtools.inspectedWindow.eval` would throw errors when used in Firefox, preventing inspect source functionality for firefox users even if they don't need to target a particular frame on the page (they are on the top level frame with Angular DevTools).

Now this behaviour is as follows:

Firefox user that has the top level frame selected -> DevTools inspect functionality works as expected.

Firefox user that has a non-top level frame selected -> DevTools now renders a snackbar message informing the user of the limitation.

PR Close #60430

devtools/projects/ng-devtools/src/lib/application-operations/BUILD.bazel

@@ -6,6 +6,7 @@ ts_library(
     name = "application-operations",
     srcs = ["index.ts"],
     deps = [
+        "//devtools/projects/ng-devtools/src/lib/application-environment",
         "//devtools/projects/protocol",
         "@npm//@types",
     ],

devtools/projects/ng-devtools/src/lib/application-operations/index.ts

@@ -6,10 +6,11 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
+import {Frame} from '../application-environment';
 import {DirectivePosition, ElementPosition} from 'protocol';
 
 export abstract class ApplicationOperations {
-  abstract viewSource(position: ElementPosition, directiveIndex?: number, target?: URL): void;
-  abstract selectDomElement(position: ElementPosition, target?: URL): void;
-  abstract inspect(directivePosition: DirectivePosition, objectPath: string[], target?: URL): void;
+  abstract viewSource(position: ElementPosition, target: Frame, directiveIndex?: number): void;
+  abstract selectDomElement(position: ElementPosition, target: Frame): void;
+  abstract inspect(directivePosition: DirectivePosition, objectPath: string[], target: Frame): void;
 }

devtools/projects/ng-devtools/src/lib/application-services/frame_manager.ts

@@ -30,6 +30,14 @@ export class FrameManager {
     return this._frames().get(selectedFrameId) ?? null;
   });
 
+  readonly topLevelFrameIsActive = computed(() => {
+    return this._selectedFrameId() === TOP_LEVEL_FRAME_ID;
+  });
+
+  readonly activeFrameHasUniqueUrl = computed(() => {
+    return this.frameHasUniqueUrl(this.selectedFrame());
+  });
+
   static initialize(inspectedWindowTabIdTestOnly?: number | null) {
     const manager = new FrameManager();
     manager.initialize(inspectedWindowTabIdTestOnly);
@@ -104,7 +112,7 @@ export class FrameManager {
     this._messageBus.emit('enableFrameConnection', [frame.id, this._inspectedWindowTabId]);
   }
 
-  frameHasUniqueUrl(frame: Frame | null): boolean {
+  private frameHasUniqueUrl(frame: Frame | null): boolean {
     if (frame === null) {
       return false;
     }

devtools/projects/ng-devtools/src/lib/application-services/frame_manager_spec.ts

@@ -169,14 +169,14 @@ describe('FrameManager', () => {
     contentScriptConnected(topLevelFrameId, 'name', 'https://angular.dev/');
     contentScriptConnected(otherFrameId, 'name2', 'https://angular.dev/');
     expect(frameManager.selectedFrame()?.url.toString()).toBe('https://angular.dev/');
-    expect(frameManager.frameHasUniqueUrl(frameManager.selectedFrame()!)).toBe(false);
+    expect(frameManager.activeFrameHasUniqueUrl()).toBe(false);
   });
 
   it('frameHasUniqueUrl should return true when only one frame has a given url', () => {
     contentScriptConnected(topLevelFrameId, 'name', 'https://angular.dev/');
     contentScriptConnected(otherFrameId, 'name', 'https://angular.dev/overview');
     expect(frameManager.selectedFrame()?.url.toString()).toBe('https://angular.dev/');
-    expect(frameManager.frameHasUniqueUrl(frameManager.selectedFrame()!)).toBe(true);
+    expect(frameManager.activeFrameHasUniqueUrl()).toBe(true);
   });
 
   it('frameHasUniqueUrl should not consider url fragments as part of the url comparison', () => {
@@ -189,10 +189,10 @@ describe('FrameManager', () => {
     expect(frameManager.selectedFrame()?.url.toString()).toBe(
       'https://angular.dev/guide/components',
     );
-    expect(frameManager.frameHasUniqueUrl(frameManager.selectedFrame()!)).toBe(false);
+    expect(frameManager.activeFrameHasUniqueUrl()).toBe(false);
   });
 
   it('frameHasUniqueUrl should return false when frame is null', () => {
-    expect(frameManager.frameHasUniqueUrl(null)).toBe(false);
+    expect(frameManager.activeFrameHasUniqueUrl()).toBe(false);
   });
 });

devtools/projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/directive-explorer.component.ts

@@ -46,6 +46,8 @@ import {PropertyTabComponent} from './property-tab/property-tab.component';
 import {SplitAreaDirective} from '../../vendor/angular-split/lib/component/splitArea.directive';
 import {MatSlideToggle} from '@angular/material/slide-toggle';
 import {FormsModule} from '@angular/forms';
+import {Platform} from '@angular/cdk/platform';
+import {MatSnackBarModule, MatSnackBar} from '@angular/material/snack-bar';
 
 const sameDirectives = (a: IndexedNode, b: IndexedNode) => {
   if ((a.component && !b.component) || (!a.component && b.component)) {
@@ -81,6 +83,7 @@ const sameDirectives = (a: IndexedNode, b: IndexedNode) => {
     PropertyTabComponent,
     MatSlideToggle,
     FormsModule,
+    MatSnackBarModule,
   ],
 })
 export class DirectiveExplorerComponent {
@@ -108,6 +111,10 @@ export class DirectiveExplorerComponent {
   private readonly _propResolver = inject(ElementPropertyResolver);
   private readonly _frameManager = inject(FrameManager);
 
+  private readonly platform = inject(Platform);
+
+  private readonly snackBar = inject(MatSnackBar);
+
   constructor() {
     afterRenderEffect((cleanup) => {
       const splitElement = this.splitElementRef().nativeElement;
@@ -134,6 +141,10 @@ export class DirectiveExplorerComponent {
     this.refresh();
   }
 
+  private isNonTopLevelFirefoxFrame() {
+    return this.platform.FIREFOX && !this._frameManager.topLevelFrameIsActive();
+  }
+
   handleNodeSelection(node: IndexedNode | null): void {
     if (node) {
       // We want to guarantee that we're not reusing any of the previous properties.
@@ -192,36 +203,42 @@ export class DirectiveExplorerComponent {
     );
 
     const selectedFrame = this._frameManager.selectedFrame();
-    if (!this._frameManager.frameHasUniqueUrl(selectedFrame)) {
-      this._messageBus.emit('log', [
-        {
-          level: 'warn',
-          message: `The currently inspected frame does not have a unique url on this page. Cannot view source.`,
-        },
-      ]);
+    if (!this._frameManager.activeFrameHasUniqueUrl()) {
+      const error = `The currently inspected frame does not have a unique url on this page. Cannot view source.`;
+      this.snackBar.open(error, 'Dismiss', {duration: 5000, horizontalPosition: 'left'});
+      this._messageBus.emit('log', [{level: 'warn', message: error}]);
       return;
     }
 
-    this._appOperations.viewSource(
-      selectedEl.position,
-      directiveIndex !== -1 ? directiveIndex : undefined,
-      new URL(selectedFrame!.url),
-    );
+    if (this.isNonTopLevelFirefoxFrame()) {
+      const error = `Viewing source is not supported in Firefox when the inspected frame is not the top-level frame.`;
+      this.snackBar.open(error, 'Dismiss', {duration: 5000, horizontalPosition: 'left'});
+      this._messageBus.emit('log', [{level: 'warn', message: error}]);
+    } else {
+      this._appOperations.viewSource(
+        selectedEl.position,
+        selectedFrame!,
+        directiveIndex !== -1 ? directiveIndex : undefined,
+      );
+    }
   }
 
   handleSelectDomElement(node: IndexedNode): void {
     const selectedFrame = this._frameManager.selectedFrame();
-    if (!this._frameManager.frameHasUniqueUrl(selectedFrame)) {
-      this._messageBus.emit('log', [
-        {
-          level: 'warn',
-          message: `The currently inspected frame does not have a unique url on this page. Cannot select DOM element.`,
-        },
-      ]);
+    if (!this._frameManager.activeFrameHasUniqueUrl()) {
+      const error = `The currently inspected frame does not have a unique url on this page. Cannot select DOM element.`;
+      this.snackBar.open(error, 'Dismiss', {duration: 5000, horizontalPosition: 'left'});
+      this._messageBus.emit('log', [{level: 'warn', message: error}]);
       return;
     }
 
-    this._appOperations.selectDomElement(node.position, new URL(selectedFrame!.url));
+    if (this.isNonTopLevelFirefoxFrame()) {
+      const error = `Inspecting a component's DOM element is not supported in Firefox when the inspected frame is not the top-level frame.`;
+      this.snackBar.open(error, 'Dismiss', {duration: 5000, horizontalPosition: 'left'});
+      this._messageBus.emit('log', [{level: 'warn', message: error}]);
+    } else {
+      this._appOperations.selectDomElement(node.position, selectedFrame!);
+    }
   }
 
   highlight(node: FlatNode): void {
@@ -291,17 +308,21 @@ export class DirectiveExplorerComponent {
     const objectPath = constructPathOfKeysToPropertyValue(node.prop);
 
     const selectedFrame = this._frameManager.selectedFrame();
-    if (!this._frameManager.frameHasUniqueUrl(selectedFrame)) {
-      this._messageBus.emit('log', [
-        {
-          level: 'warn',
-          message: `The currently inspected frame does not have a unique url on this page. Cannot inspect object.`,
-        },
-      ]);
+
+    if (!this._frameManager.activeFrameHasUniqueUrl()) {
+      const error = `The currently inspected frame does not have a unique url on this page. Cannot inspect object.`;
+      this.snackBar.open(error, 'Dismiss', {duration: 5000, horizontalPosition: 'left'});
+      this._messageBus.emit('log', [{level: 'warn', message: error}]);
       return;
     }
 
-    this._appOperations.inspect(directivePosition, objectPath, new URL(selectedFrame!.url));
+    if (this.isNonTopLevelFirefoxFrame()) {
+      const error = `Inspecting object is not supported in Firefox when the inspected frame is not the top-level frame.`;
+      this.snackBar.open(error, 'Dismiss', {duration: 5000, horizontalPosition: 'left'});
+      this._messageBus.emit('log', [{level: 'warn', message: error}]);
+    } else {
+      this._appOperations.inspect(directivePosition, objectPath, selectedFrame!);
+    }
   }
 
   hightlightHydrationNodes() {

devtools/projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/directive-explorer.spec.ts

@@ -260,8 +260,8 @@ describe('DirectiveExplorerComponent', () => {
         expect(messageBusMock.emit).toHaveBeenCalledWith('enableFrameConnection', [0, 123]);
         expect(applicationOperationsSpy.viewSource).toHaveBeenCalledWith(
           [0], // current selected element position
+          {name: 'test1', id: 0, url: new URL('http://localhost:4200/url')},
           0, // directive index
-          new URL('http://localhost:4200/url'), // selected frame url
         );
       });
     });
@@ -298,7 +298,7 @@ describe('DirectiveExplorerComponent', () => {
         expect(messageBusMock.emit).toHaveBeenCalledWith('enableFrameConnection', [0, 123]);
         expect(applicationOperationsSpy.selectDomElement).toHaveBeenCalledWith(
           [0], // current selected element position
-          new URL('http://localhost:4200/url'), // selected frame url
+          {name: 'test1', id: 0, url: new URL('http://localhost:4200/url')},
         );
       });
     });
@@ -355,11 +355,11 @@ describe('DirectiveExplorerComponent', () => {
 
         expect(applicationOperationsSpy.inspect).toHaveBeenCalledTimes(1);
         expect(messageBusMock.emit).toHaveBeenCalledWith('enableFrameConnection', [0, 123]);
-        expect(applicationOperationsSpy.inspect).toHaveBeenCalledWith(
-          directivePosition,
-          ['foo'],
-          new URL('http://localhost:4200/url'), // selected frame url
-        );
+        expect(applicationOperationsSpy.inspect).toHaveBeenCalledWith(directivePosition, ['foo'], {
+          name: 'test1',
+          id: 0,
+          url: new URL('http://localhost:4200/url'),
+        });
       });
     });
   });

devtools/projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/property-tab/property-view/property-view-header.component.html

@@ -1,6 +1,10 @@
 <mat-toolbar>
   {{ directive() }}
-  <button matTooltip="Open source" (click)="handleViewSource($event)">
+  <button
+    [disabled]="disableViewSourceButton()"
+    [matTooltip]="disableViewSourceButton() ? 'Inspecting source is not supported in Firefox when the inspected frame is not the top-level frame.' : 'Open Source'"
+    (click)="handleViewSource($event)"
+  >
     <mat-icon> code </mat-icon>
   </button>
 </mat-toolbar>

devtools/projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/property-tab/property-view/property-view-header.component.scss

@@ -30,6 +30,11 @@ button {
   &:active {
     opacity: 1;
   }
+
+  &:disabled {
+    cursor: not-allowed;
+    opacity: 1;
+  }
 }
 
 :host-context(.dark-theme) {

devtools/projects/ng-devtools/src/lib/devtools-tabs/directive-explorer/property-tab/property-view/property-view-header.component.ts

@@ -6,10 +6,12 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {Component, input, output} from '@angular/core';
+import {Component, computed, inject, input, output} from '@angular/core';
 import {MatIcon} from '@angular/material/icon';
 import {MatTooltip} from '@angular/material/tooltip';
 import {MatToolbar} from '@angular/material/toolbar';
+import {Platform} from '@angular/cdk/platform';
+import {FrameManager} from '../../../../application-services/frame_manager';
 
 @Component({
   selector: 'ng-property-view-header',
@@ -21,6 +23,15 @@ export class PropertyViewHeaderComponent {
   readonly directive = input.required<string>();
   readonly viewSource = output<void>();
 
+  private readonly frameManager = inject(FrameManager);
+  private readonly platform = inject(Platform);
+
+  readonly disableViewSourceButton = computed(() => {
+    const isTopLevelFrame = this.frameManager.topLevelFrameIsActive();
+    const frameHasUniqueUrl = this.frameManager.activeFrameHasUniqueUrl();
+    return (this.platform.FIREFOX && !isTopLevelFrame) || !frameHasUniqueUrl;
+  });
+
   // output that emits directive
   handleViewSource(event: MouseEvent): void {
     event.stopPropagation();

devtools/projects/shell-browser/src/app/BUILD.bazel

@@ -35,6 +35,7 @@ ng_module(
         "//packages/core",
         "//packages/platform-browser",
         "//packages/platform-browser/animations",
+        "@npm//@angular/cdk",
         "@npm//@angular/material",
         "@npm//rxjs",
     ],
@@ -85,6 +86,7 @@ ts_library(
         "//devtools/projects/ng-devtools",
         "//devtools/projects/protocol",
         "//packages/core",
+        "@npm//@angular/cdk",
         "@npm//@types",
     ],
 )

devtools/projects/shell-browser/src/app/app.config.ts

@@ -15,6 +15,7 @@ import {ChromeApplicationOperations} from './chrome-application-operations';
 import {ZoneAwareChromeMessageBus} from './zone-aware-chrome-message-bus';
 import {Events, MessageBus, PriorityAwareMessageBus} from 'protocol';
 import {FrameManager} from '../../../ng-devtools/src/lib/application-services/frame_manager';
+import {Platform} from '@angular/cdk/platform';
 
 export const appConfig: ApplicationConfig = {
   providers: [
@@ -23,6 +24,7 @@ export const appConfig: ApplicationConfig = {
     {
       provide: ApplicationOperations,
       useClass: ChromeApplicationOperations,
+      deps: [Platform],
     },
     {
       provide: ApplicationEnvironment,

devtools/projects/shell-browser/src/app/chrome-application-operations.ts

@@ -8,25 +8,44 @@
 
 /// <reference types="chrome"/>
 
-import {ApplicationOperations} from 'ng-devtools';
+import {Platform} from '@angular/cdk/platform';
+import {inject} from '@angular/core';
+import {ApplicationOperations, Frame, TOP_LEVEL_FRAME_ID} from 'ng-devtools';
 import {DirectivePosition, ElementPosition} from 'protocol';
 
-function runInInspectedWindow(script: string, frameURL?: URL): void {
-  chrome.devtools.inspectedWindow.eval(script, {frameURL: frameURL?.toString?.()});
-}
-
 export class ChromeApplicationOperations extends ApplicationOperations {
-  override viewSource(position: ElementPosition, directiveIndex?: number, target?: URL): void {
+  platform = inject(Platform);
+
+  private runInInspectedWindow(script: string, target: Frame) {
+    if (this.platform.FIREFOX && target.id !== TOP_LEVEL_FRAME_ID) {
+      console.error(
+        '[Angular DevTools]: This browser does not support targeting a specific frame for eval by URL.',
+      );
+      return;
+    } else if (this.platform.FIREFOX) {
+      chrome.devtools.inspectedWindow.eval(script);
+      return;
+    }
+
+    const frameURL = target.url;
+    chrome.devtools.inspectedWindow.eval(script, {frameURL: frameURL?.toString?.()});
+  }
+
+  override viewSource(position: ElementPosition, target: Frame, directiveIndex?: number): void {
     const viewSource = `inspect(inspectedApplication.findConstructorByPosition('${position}', ${directiveIndex}))`;
-    runInInspectedWindow(viewSource, target);
+    this.runInInspectedWindow(viewSource, target);
   }
 
-  override selectDomElement(position: ElementPosition, target?: URL): void {
+  override selectDomElement(position: ElementPosition, target: Frame): void {
     const selectDomElement = `inspect(inspectedApplication.findDomElementByPosition('${position}'))`;
-    runInInspectedWindow(selectDomElement, target);
+    this.runInInspectedWindow(selectDomElement, target);
   }
 
-  override inspect(directivePosition: DirectivePosition, objectPath: string[], target?: URL): void {
+  override inspect(
+    directivePosition: DirectivePosition,
+    objectPath: string[],
+    target: Frame,
+  ): void {
     const args = {
       directivePosition,
       objectPath,
@@ -34,6 +53,6 @@ export class ChromeApplicationOperations extends ApplicationOperations {
     const inspect = `inspect(inspectedApplication.findPropertyByPosition('${JSON.stringify(
       args,
     )}'))`;
-    runInInspectedWindow(inspect, target);
+    this.runInInspectedWindow(inspect, target);
   }
 }