1296870 - console.clear makes the Browser Console unusable

Status: VERIFIED FIXED

(DevTools :: Console, defect, P1)

Description

[Andreas Jung]

If console.clear is called in a loop (and I've seen ad-iframes do this) the browser console becomes completely unusable as everything from logs to errors to security messages disappears immediately.

The worst part is there is no way to find out which tab / iframe even caused this.

I think console.clear should only clear the web console and leave the browser console untouched.

Comment 1

[BoffinBrain]

Attachment: Easy repro case in HTML

As expected, ruining a developer's day is as easy as writing a single line of code:

>  window.setInterval(console.clear, 10);

Given that this technique can be used to suppress security warnings and security errors, I wonder if it could be classified as a security vulnerability?

I really wish the devs would consider the consequences of letting page authors tamper with the browser before adding more 'features' like this in future.

For possible solutions, please see my previous comment on the parent bug: https://bugzilla.mozilla.org/show_bug.cgi?id=659625#c68

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Andreas Jung from comment #0)
> If console.clear is called in a loop (and I've seen ad-iframes do this) the
> browser console becomes completely unusable as everything from logs to
> errors to security messages disappears immediately.
> 
> The worst part is there is no way to find out which tab / iframe even caused
> this.
> 
> I think console.clear should only clear the web console and leave the
> browser console untouched.

I agree that we should make console.clear have to effect on the Browser Console

Comment 3

[Brian Grinstead [:bgrins]]

Typo: 'have to' -> 'have no'

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1296870 - Rename browser_console clear test to browser_webconsole;

Review commit: https://reviewboard.mozilla.org/r/73408/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73408/

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1296870 - Do not clear output of BrowserConsole when receiving console.clear();

Review commit: https://reviewboard.mozilla.org/r/73410/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73410/

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8783711 [details]
Bug 1296870 - Rename browser_console clear test to browser_webconsole;

https://reviewboard.mozilla.org/r/73408/#review71232

Comment 7

[Brian Grinstead [:bgrins]]

Comment on attachment 8783712 [details]
Bug 1296870 - Do not clear output of BrowserConsole when receiving console.clear();

https://reviewboard.mozilla.org/r/73410/#review71234

::: devtools/client/webconsole/webconsole.js:2170
(Diff revision 1)
>      let isRepeated = this._filterRepeatedMessage(node);
>  
>      // If a clear message is processed while the webconsole is opened, the UI
>      // should be cleared.
> -    if (message && message.level == "clear") {
> +    // Do not clear the output if the current frame is owned by a Browser Console.
> +    if (message && message.level == "clear" && !this.owner.isBrowserConsole()) {

`this.owner._browserConsole` is used throughout this file.  I think it'd be nicer if we set a value on the WebConsoleFrame in the constructor that reaches up to `this.owner._browserConsole`.

So inside WebConsoleFrame constructor:

`this.isBrowserConsole = this.owner._browserConsole`

Then replace anything that's referencing `*.owner._browserConsole` in this file and jsterm.js with `*.isBrowserConsole`.  I don't think the new function in hudservice is necessary since the only callers are here and jsterm.js (and would be reduced down to just one place in this plan).

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8783712 [details]
Bug 1296870 - Do not clear output of BrowserConsole when receiving console.clear();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/73410/diff/1-2/

Comment 9

[Julian Descottes [:jdescottes]]

Thanks for the reviews! Applied your comments and pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc9e4067e011

Comment 10

[Brian Grinstead [:bgrins]]

Comment on attachment 8783712 [details]
Bug 1296870 - Do not clear output of BrowserConsole when receiving console.clear();

https://reviewboard.mozilla.org/r/73410/#review71424

Works for me, thanks!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37020799f784
Rename browser_console clear test to browser_webconsole;r=bgrins
https://hg.mozilla.org/integration/autoland/rev/0c21a0e9bca2
Do not clear output of BrowserConsole when receiving console.clear();r=bgrins

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/37020799f784
https://hg.mozilla.org/mozilla-central/rev/0c21a0e9bca2

Comment 13

[Tanvir Rahman]

I have reproduced this bug with Nightly 51.0a1 (2016-08-20) on Windows 10, 64 bit!

The Bug's fix is now verified on Latest Beta 51.0b12

Build ID 	20170105155013
User Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0

[testday-20170106]

Comment 14

[Bogdan Maris, Desktop QA]

(In reply to Tanvir Rahman from comment #13)
> I have reproduced this bug with Nightly 51.0a1 (2016-08-20) on Windows 10,
> 64 bit!
> 
> The Bug's fix is now verified on Latest Beta 51.0b12
> 
> Build ID 	20170105155013
> User Agent 	Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101
> Firefox/51.0
> 
> [testday-20170106]

Thanks for verifying!