Closed Bug 981241 Opened 6 years ago Closed 6 years ago

Make the CSS category in the web console more flexible

Categories

(DevTools :: Console, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 30

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(1 file, 1 obsolete file)

I noticed using "CSS" as a category when calling ReportToConsole does not actually categorize the message as a CSS message. After asking around, I was pointed to this code:

https://mxr.mozilla.org/mozilla-central/source/browser/devtools/webconsole/webconsole.js#4545

I think we could use a little more flexibility here. At a minimum, I'd like to add "CSS" and "Layout" to the list of strings that result in CATEGORY_CSS. It might be even better, though, to categorize any string that begins with "CSS[ :]" or "Layout[ :]" as CATEGORY_CSS.
We've been fairly picky about categories to avoid 'leaking' unrelated messages into the webconsole (we already have too many messages being logged).
Summary: Make the categories in the web console more flexible → Make the CSS category in the web console more flexible
Mihai and I talked on IRC and clarified that a patch like this should be OK.

I decided to avoid broadening the scope of this bug too much, given that I don't really know the needs of e.g. the security people, so this patch just adds additional flexibility for CSS and layout-related web console messages.
Attachment #8388806 - Flags: review?(mihai.sucan)
Blocks: 981924
Comment on attachment 8388806 [details] [diff] [review]
Make the CSS category in the web console more flexible.

Review of attachment 8388806 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks!

::: browser/devtools/webconsole/webconsole.js
@@ +4552,5 @@
>     *         CATEGORY_SECURITY can be returned.
>     */
>    categoryForScriptError: function Utils_categoryForScriptError(aScriptError)
>    {
> +    var category = aScriptError.category;

nit: s/var/let/ for consistency

@@ +4555,5 @@
>    {
> +    var category = aScriptError.category;
> +
> +    if (/^CSS([: ].+|$)/.test(category) ||
> +        /^Layout([: ].+|$)/.test(category)) {

These do not need to be capturing regular expressions.

How about /^(?:CSS|Layout)\b/.test(category) ?
Attachment #8388806 - Flags: review?(mihai.sucan) → review+
Thanks for the review!

(In reply to Mihai Sucan [:msucan] from comment #3)
> How about /^(?:CSS|Layout)\b/.test(category) ?

That sounds good to me; will update the patch.
Attachment #8388806 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/c1da921cae5a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
QA Whiteboard: [qa-]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.