Closed Bug 1674345 Opened 4 years ago Closed 3 years ago

Emit Console API disabled message from the devtools server or directly from platform

Categories

(DevTools :: Console, task, P2)

task

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Details

Attachments

(1 obsolete file)

At the moment, when the console object is overridden by a script on a page, we display a warning message for the user.
This is done by checking if the global console object in the content page is the native one. We then pass this information as part of the startListeners response, and the client is then responsible to display the warning message.
This is not ideal, and we can probably emit this message from the server/platform.

Whiteboard: dt-fission-m3-mvp

Tracking dt-fission-m3-mvp bugs for Fission M8 (blocking Release channel experiment, but not Beta experiment).

Fission Milestone: --- → M8

Jason, I was told you could be of some assistance here.

In DevTools, we want to display a warning message to the user if the global.console object is overridden by a script on the page.
At the moment we're doing this from DevTools code, when opening the toolbox and when navigating, by checking if the console object is the native one (devtools/server/actors/webconsole.js#386-406).

So here there's multiple issue:

  • if the console object is overridden by a script while the console was still open, we won't send a warning message
  • we can't do this check for worker globals (because we can't use XPCNativeWrapper there)
  • our setup is super weird as the client is then responsible for emitting the message, whereas for regular warning messages, we're simply consuming nsIScriptErrors

So I was wondering if we could emit this message directly from the platform, like we do for so many other warning messages (e.g. Deprecated Warnings messages when accessing a deprecated property like Document#onmozfullscreen).
I'm willing to tackle this, but I have no idea where to start.
Where should I look in the code to detect when the global console property is set?

Thanks!

Flags: needinfo?(jorendorff)
Summary: Emit Console API disabled message from the server → Emit Console API disabled message from the devtools server or directly from platform

To answer your question: I don't think there is a sound way to get notified when a script on a page modifies console, but you can prevent future changes: use Debugger.Object.freeze on the console object, and use Debugger.Object.defineProperty to make global.console non-configurable and non-writable.

However, I'm a little concerned about this warning. Is this a security feature? What's the situation we're warning against, specifically?

Flags: needinfo?(jorendorff) → needinfo?(nchevobbe)

Here's the message we print in the console:

The Web Console logging API (console.log, console.info, console.warn, console.error) has been disabled by a script on this page.

(https://searchfox.org/mozilla-central/rev/03224522336f60a1a61a87e1fcd4feb0a0315a9b/devtools/client/locales/en-US/webconsole.properties#25)

The idea is that if a user want to debug a page but a library/script overrode it, at least the user would know why they don't see their logs.
I don't think we want to freeze the console object, just report when it occurs.

Flags: needinfo?(nchevobbe)

OK, cool. I confirmed with a buddy—there's no way to get a notification without modifying the console property yourself (in a way that could conceivably break something on the page).

It's probably best to check again—maybe a second after each time a script loads. I know, gross :-P

Okay, thank you for checking Jason

removing from Fission scope

No longer blocks: dt-fission-console
Whiteboard: dt-fission-m3-mvp

In order to warn the user about overridden console API, we used to include a nativeConsoleAPI
property as a result of the startListeners method on the console actor. The client
would then check that property and if it was false, emit a warning message.

This is a bit convoluted, so this patch remove the nativeConsoleAPI property (as
well as one that was send as a trait from the browsing context actor), and instead
emit the message directly from the server if need be.

Since actors can be instanciated multiple times, we keep an array of the window
ids for which we already emitted the warning message.

We still need to keep the client code so we don't break bacward compatibility;
the code is commented with the backward-compat keyword so we can cleanup this
when 86 reaches release.

Some tests are modified since they were checking the nativeConsoleAPI property/trait,
which isn't there anymore.

Whiteboard: dt-fission-m3-triage
Attachment #9197686 - Attachment is obsolete: true

From discussion on the patch + work happening in Bug 1712591 , I think we can close this.
We'll still emit the message from the client, but the information for overrided console object will be part of a DOCUMENT_EVENT resource which is cleaner that what we have now

No longer blocks: 1674342
Status: NEW → RESOLVED
Fission Milestone: M8 → ---
Closed: 3 years ago
Resolution: --- → INVALID
Whiteboard: dt-fission-m3-triage
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: