[Webkit-unassigned] [Bug 44518] Web Inspector: add audits support to extension API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44518


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #65263|review?                     |review-
               Flag|                            |




--- Comment #2 from Pavel Feldman <pfeldman at chromium.org>  2010-08-25 02:23:16 PST ---
(From update of attachment 65263)
Overall looks good, few nits below. r- for naming + formatter.

LayoutTests/inspector/extensions-audits-expected.txt:16
 +      cancel : <function>
cancel -> done

LayoutTests/inspector/extensions-audits.html:11
 +      function onStartAuditCategory(auditRun)
auditRun -> testResult

WebCore/inspector/front-end/AuditLauncherView.js:138
 +              insertBefore < this._sortedCategories.length && this._compareCategories(category, this._sortedCategories[insertBefore]) >= 0;
Please fix indentation.

WebCore/inspector/front-end/AuditLauncherView.js:142
 +              this._categoriesElement.insertBefore(categoryElement, this._categoriesElement.children[insertBefore]);
insertBefore undefined will do appendChild, no need to fork the code.

WebCore/inspector/front-end/AuditLauncherView.js:130
 +          categoryElement.firstChild.addEventListener("click", this._boundCategoryClickListener, false);
you could encapsulate this listener addition into the _createCategoryElement in order not to be involved with the internal structure here.

WebCore/inspector/front-end/AuditLauncherView.js:137
 +          for (var insertBefore = 0;
I just found indexOfObjectInListSortedByFunction in the utilities.js

WebCore/inspector/front-end/ExtensionAPI.js:50
 +          if (typeof callback != "function")
!==

WebCore/inspector/front-end/ExtensionAuditCategory.js:55
 +      runRules: function(resources, callback)
runRules -> run ?

WebCore/inspector/front-end/ExtensionAuditCategory.js:83
 +      addRuleResult: function(ruleDisplayName, severity, violationCount, details)
addResult ?

LayoutTests/inspector/extensions-audits.html:24
 +              type: webInspector.audits.nodeType.text,
I'd suggest that we use the following form text parameters:
["Foo bar %h", "http://link"].
"children" should still be there in order to organize a very common tree structure.
We could come up with more formatters as we go:
%s for string
%t for tree
%T for table and such...

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.