Add errors count to the Web Console button in the developer toolbar

RESOLVED FIXED in Firefox 16

Status

()

Firefox
Developer Tools: Console
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: msucan, Assigned: msucan)

Tracking

Trunk
Firefox 16
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [devtb])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Add errors count to the Web Console button in the developer toolbar.
(Assignee)

Comment 1

5 years ago
Created attachment 632786 [details] [diff] [review]
[in-fx-team] proposed patch

This is the proposed patch with a test included. The Web Console code needed really minimal changes to make sure that both the dev toolbar and the Web Console can reuse the same HUDService-content.js script at the same time.

Paul: please let me know if you can also review this code or if I should also ask someone else for a review. Thank you!

I would also appreciate your UI magic on this patch. Currently I only show the errors count like this: "Web Console (N)". If you have time, please make a patch on top of this one that changes how the errors count is displayed. Or please give me UI suggestions on how you'd like this be - I can implement it if you are busy. Thank you very much!


Try push:
https://tbpl.mozilla.org/?tree=Try&rev=2768240e842e
Attachment #632786 - Flags: feedback?(paul)
(Assignee)

Comment 2

5 years ago
Do note the Web Console patch queue I currently have:
https://github.com/mihaisucan/mozilla-patch-queue/blob/master/patches-webconsole/series
Depends on: 761257

Comment 3

5 years ago
Comment on attachment 632786 [details] [diff] [review]
[in-fx-team] proposed patch

This is an awesome feature :) Thank you Mihai.

Nits, not blocking:

+  if (errors) {
+    this._webConsoleButton.label =
+      this._webConsoleButtonLabel + " (" + errors + ")";
+  }

Can you add a <label> element in the button instead?
(I can do it later if you want)

+DeveloperToolbar.prototype._onPageError =
+function DT__onPageError(aTabId, aPageError)
+{
+  if (aPageError.category == "CSS Parser" ||
+      aPageError.category == "CSS Loader" ||
+      (aPageError.flags & aPageError.warningFlag) ||
+      (aPageError.flags & aPageError.strictFlag)) {
+    return; // just a CSS or JS warning
+  }
+
+  this._errorsCount[aTabId]++;
+};

Instead of a black list, can we have a white list?
For clarity and in case we add new kind of logs in the future.
Attachment #632786 - Flags: feedback?(paul) → review+

Updated

5 years ago
Blocks: 764746
(Assignee)

Comment 4

5 years ago
(In reply to Paul Rouget [:paul] from comment #3)
> Comment on attachment 632786 [details] [diff] [review]
> proposed patch
> 
> This is an awesome feature :) Thank you Mihai.

Thanks! I'm glad you like it.

> Nits, not blocking:
> 
> +  if (errors) {
> +    this._webConsoleButton.label =
> +      this._webConsoleButtonLabel + " (" + errors + ")";
> +  }
> 
> Can you add a <label> element in the button instead?
> (I can do it later if you want)

Sure!

> +DeveloperToolbar.prototype._onPageError =
> +function DT__onPageError(aTabId, aPageError)
> +{
> +  if (aPageError.category == "CSS Parser" ||
> +      aPageError.category == "CSS Loader" ||
> +      (aPageError.flags & aPageError.warningFlag) ||
> +      (aPageError.flags & aPageError.strictFlag)) {
> +    return; // just a CSS or JS warning
> +  }
> +
> +  this._errorsCount[aTabId]++;
> +};
> 
> Instead of a black list, can we have a white list?
> For clarity and in case we add new kind of logs in the future.

Good point, but this copies the way the Web Console works. We should keep these in sync. These PageError messages are nsIScriptErrors we receive from HUDService-content.js.

Thank you for the r+!
(Assignee)

Comment 5

5 years ago
Comment on attachment 632786 [details] [diff] [review]
[in-fx-team] proposed patch

https://hg.mozilla.org/integration/fx-team/rev/59c5ee47dc45
Attachment #632786 - Attachment description: proposed patch → [in-fx-team] proposed patch
(Assignee)

Comment 6

5 years ago
Tried adding a label element into the toolbarbutton as suggested, but it doesn't show up. Leaving style work to happen in bug 764746. Thank you!
Whiteboard: [devtb][fixed-in-fx-team]
Duplicate of this bug: 764852
https://hg.mozilla.org/mozilla-central/rev/59c5ee47dc45
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Whiteboard: [devtb][fixed-in-fx-team] → [devtb]
You need to log in before you can comment on or make changes to this bug.