Closed Bug 592879 Opened 11 years ago Closed 11 years ago

Detect and warn users that the current window.console API is third party

Categories

(DevTools :: General, defect)

x86
All
defect
Not set
normal

Tracking

(blocking2.0 beta7+)

RESOLVED FIXED
Firefox 4.0b7
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: ddahl, Assigned: ddahl)

References

Details

(Whiteboard: [strings] [kd4b6])

Attachments

(1 file, 4 obsolete files)

detect a non-firefox console and respectfully leave it alone, adding an INFO or WARN message to the console about this.

let the user know that:

 "we have detected a 3rd party window.console API, and any calls to console.info/warn/log/error will not be displayed in the webconsole"

or something to that effect.
Assignee: nobody → ddahl
Blocks: 529086
Whiteboard: [string]
blocking2.0: --- → ?
Whiteboard: [string] → [strings]
This bug is split off of bug 583476, which is specifically covering an issue in which the console does not activate on some sites. As a separate issue, this bug addresses the need to tell the user when Firefox's built-in console object has been overridden by JavaScript provided on the page.
Whiteboard: [strings] → [strings] [kd4b6]
For beta 6 I am going to write the method that does the warning part and create the string, as the detection part will have to wait until after string freeze
I would recommend producing that patch as soon as possible...
Severity: normal → blocker
Attached patch v 0.0 Saving work (obsolete) — Splinter Review
the warning/error message I have so far for this string is:

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

Sounds good? makes sense? is there a better message?
Attached patch v 0.1 Saving Work (obsolete) — Splinter Review
Attachment #471938 - Attachment is obsolete: true
Attached patch v 0.2 Patch (obsolete) — Splinter Review
Part 1 of 2 patches, the second of which will detect the page-created browser. This patch needs to land for b6, the second can land before final.
Attachment #471945 - Attachment is obsolete: true
Attachment #471985 - Flags: review?(dietrich)
Attachment #471985 - Flags: feedback+
Does this obsolete the first patch ("patch v1.2") in bug 583476?
(In reply to comment #8)
> Does this obsolete the first patch ("patch v1.2") in bug 583476?

Not sure. I do think that the problem is not that there is a new console, rather, there are some exceptions that are thrown preventing us from opeing the UI. It may or may no thave to do with the paved over console.

Still working on that aspect of it.
Comment on attachment 471985 [details] [diff] [review]
v 0.2 Patch


>+  logWarningAboutReplacedAPI:
>+  function HS_logWarningAboutReplacedAPI(aHUDId)
>+  {
>+    let domId = "hud-log-node-" + this.sequenceId();
>+    let displayNode = this.getHeadsUpDisplay(aHUDId);
>+    let outputNode = displayNode.querySelectorAll(".hud-output-node")[0];

see this a lot. instead of hard-coding the selector everywhere, maybe should make a convenience function this.getHUDOutputNode(aHUDId);

r=me otherwise.
Attachment #471985 - Flags: review?(dietrich) → review+
Reprioritizing bugs. You can filter the mail on the word TEABAGS.
Severity: blocker → normal
Assignee: ddahl → jwalker
(In reply to comment #11)
> Reprioritizing bugs. You can filter the mail on the word TEABAGS.

huh? I do not understand what you are doing here. the first patch is a b6 blocker with strings.
It appears that severity=blocker means something different to other people (see the bug writing guidelines doc) and does not mean the same thing as blocker2.0. At the end of my machinations here, I think I'll have something clearer for the team anyhow.
Blocks: devtools4b7
Mihai has included this change with slightly different wording in bug 583476.
Moving [strings] patches between bugs of different triage status isn't making triage easier, FWIW.
(In reply to comment #14)
> Mihai has included this change with slightly different wording in bug 583476.

The reason I have been asking mihai not to worry so much about this problem is virtually all of this code will be ripped out next week. The problems he should look into are the exceptions that are raised when trying to display the UI.

This is unfair to localizers and is not cool to me as I spent some time on this patch last week - and - it is isolated and easy to land. Enough with the refactoring already. This is not going to affect that many people and is better dealt with when we are not under string freeze.
Passes all tests
Attachment #471985 - Attachment is obsolete: true
Attachment #473075 - Flags: review+
Assignee: jwalker → ddahl
blocking2.0: ? → beta6+
David and Axel: I agree that bouncing that string between the bugs is problematic. Mihai is changing bug 583476 to depend on this one for the string.

(That said, it looks like the progress on bug 583476 has been good so we can have the console popup on problematic sites in beta 6.)
There's a bug in the latest patch:

  getConsoleOutputNode: function HS_getConsoleOutputNode(aId)
  {
    let displayNode = this.getHeadsUpDisplay(aHUDId);
    return displayNode.querySelectorAll(".hud-output-node")[0];
  },

aHUDId is undefined.
Attached patch v 0.4 updated with a fix (obsolete) — Splinter Review
Updated patch to fix the issue reported above.
Attachment #473075 - Attachment is obsolete: true
Blocks: 583476
please file a followup bug for the fix. this patch is already reviewed and approved.
Attachment #473075 - Attachment is obsolete: false
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/6bed6915317a
Status: NEW → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b6
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Backed out
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Actually, there is one more patch coming from bug 583476
Comment on attachment 473166 [details] [diff] [review]
v 0.4 updated with a fix

Marking this patch as obsolete, because 583476 will include the fix.
Attachment #473166 - Attachment is obsolete: true
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
the test for this patch leaked on tinderbox.
Correct. I have a fix for this, in the upcoming patch for bug 583476.
http://hg.mozilla.org/mozilla-central/rev/e931499f28eb
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.