console.clear makes the Browser Console unusable

VERIFIED FIXED in Firefox 51

Status

()

Firefox
Developer Tools: Console
P1
normal
VERIFIED FIXED
9 months ago
5 months ago

People

(Reporter: Andreas Jung, Assigned: jdescottes)

Tracking

48 Branch
Firefox 51
Points:
---

Firefox Tracking Flags

(firefox51 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

9 months ago
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.
(Reporter)

Updated

9 months ago
Depends on: 659625

Comment 1

9 months ago
Created attachment 8783632 [details]
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
(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
Severity: major → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P1
Typo: 'have to' -> 'have no'
(Assignee)

Updated

9 months ago
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

9 months ago
mozreview-review
Comment on attachment 8783711 [details]
Bug 1296870 - Rename browser_console clear test to browser_webconsole;

https://reviewboard.mozilla.org/r/73408/#review71232
Attachment #8783711 - Flags: review?(bgrinstead) → review+

Comment 7

9 months ago
mozreview-review
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).
Attachment #8783712 - Flags: review?(bgrinstead)
Comment hidden (mozreview-request)
(Assignee)

Comment 9

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

Comment 10

9 months ago
mozreview-review
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!
Attachment #8783712 - Flags: review?(bgrinstead) → review+

Comment 11

9 months ago
Pushed by jdescottes@mozilla.com:
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

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/37020799f784
https://hg.mozilla.org/mozilla-central/rev/0c21a0e9bca2
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51

Comment 13

5 months ago
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]
(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!
Status: RESOLVED → VERIFIED
status-firefox51: fixed → verified
You need to log in before you can comment on or make changes to this bug.