Bug 35860 – Web Inspector: Refactor Audits panel presentation layer.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

This change removes unnecessary complexity:
- Audit scores were not used
- Audit rule parameters are passed as rule constructor arguments
- View management aligned with the rest of the front-end
- Single TreeOutline is used for category results (no need to create sections for those)
- Rules code beautified and simplified where necessary
- Some UI improvements applied (see attached screenshot)

There are some outstanding issues preventing us from enabling audits by default:
1. Toolbar icon
2. We should probbaly enable audits based on the Preferences (the feature is new, needs some testing and feedback)
3. CSS-related audits are not correct: they assume you can iterate stylesheet objects using cssRules which is not truth for external styles. Hints for this one are welcome.

[Pavel Feldman]

Created attachment 50207 [details]
[IMAGE] Audit launcher panel.

[Pavel Feldman]

Created attachment 50208 [details]
[IMAGE] Audits run results for CNN (default unexpanded view).

[Pavel Feldman]

Created attachment 50209 [details]
[IMAGE] Hovercard demo results expanded.

[Pavel Feldman]

Created attachment 50211 [details]
[PATCH] Proposed change.

[Timothy Hatcher]

Comment on attachment 50211 [details]
[PATCH] Proposed change.

> +        summary.value = "The following domains only serve one resource each. If possible, avoid the extra DNS " +
> +            "lookups by serving these resources from existing domains.";

Just make this be on one line. It will need to be later for i18n anyway.

Is there a P1 bug about i18n? I worry we will have trouble finding all the strings later…

I will work on an icon sometime today.

[Pavel Feldman]

(In reply to comment #5)
> (From update of attachment 50211 [details])
> > +        summary.value = "The following domains only serve one resource each. If possible, avoid the extra DNS " +
> > +            "lookups by serving these resources from existing domains.";
> 
> Just make this be on one line. It will need to be later for i18n anyway.
> 

Done.

> Is there a P1 bug about i18n? I worry we will have trouble finding all the
> strings later…

Whenever we enable it, we should do P1. FYI: we are not localizing devtools in Chromium. Are you sure we need that for audits?

> 
> I will work on an icon sometime today.

[Timothy Hatcher]

(In reply to comment #6)
> (In reply to comment #5)
> > Is there a P1 bug about i18n? I worry we will have trouble finding all the
> > strings later…
> 
> Whenever we enable it, we should do P1. FYI: we are not localizing devtools in
> Chromium. Are you sure we need that for audits?

We localize the Web Inspector in Safari releases. If the Audits panel is enabled, it needs to be localizable. Or we will need to potentially disable it in Safari.

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/AuditCategories.js
	M	WebCore/inspector/front-end/AuditResultView.js
	M	WebCore/inspector/front-end/AuditRules.js
	M	WebCore/inspector/front-end/AuditsPanel.js
	M	WebCore/inspector/front-end/Settings.js
	M	WebCore/inspector/front-end/audits.css
	M	WebCore/inspector/front-end/inspector.js
Committed r55727