Bug 35860 - Web Inspector: Refactor Audits panel presentation layer.
Summary: Web Inspector: Refactor Audits panel presentation layer.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-08 04:29 PST by Pavel Feldman
Modified: 2010-03-09 09:35 PST (History)
5 users (show)

See Also:


Attachments
[IMAGE] Audit launcher panel. (113.21 KB, image/png)
2010-03-08 04:30 PST, Pavel Feldman
no flags Details
[IMAGE] Audits run results for CNN (default unexpanded view). (138.37 KB, image/png)
2010-03-08 04:31 PST, Pavel Feldman
no flags Details
[IMAGE] Hovercard demo results expanded. (249.05 KB, image/png)
2010-03-08 04:32 PST, Pavel Feldman
no flags Details
[PATCH] Proposed change. (84.03 KB, patch)
2010-03-08 04:37 PST, Pavel Feldman
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2010-03-08 04:29:46 PST
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.
Comment 1 Pavel Feldman 2010-03-08 04:30:45 PST
Created attachment 50207 [details]
[IMAGE] Audit launcher panel.
Comment 2 Pavel Feldman 2010-03-08 04:31:29 PST
Created attachment 50208 [details]
[IMAGE] Audits run results for CNN (default unexpanded view).
Comment 3 Pavel Feldman 2010-03-08 04:32:25 PST
Created attachment 50209 [details]
[IMAGE] Hovercard demo results expanded.
Comment 4 Pavel Feldman 2010-03-08 04:37:48 PST
Created attachment 50211 [details]
[PATCH] Proposed change.
Comment 5 Timothy Hatcher 2010-03-09 08:37:05 PST
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.
Comment 6 Pavel Feldman 2010-03-09 08:56:18 PST
(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.
Comment 7 Timothy Hatcher 2010-03-09 09:01:29 PST
(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.
Comment 8 Pavel Feldman 2010-03-09 09:35:28 PST
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