document.defaultView.getMatchedCSSRules should be replaced with InspectorBackend accessor.
Created attachment 51284 [details] [PATCH] Proposed solution
Comment on attachment 51284 [details] [PATCH] Proposed solution > + function imageStylesReady(imageId, context, styles) > + { > + var widthFound, heightFound; > + --context.imagesLeft; > + > + const node = WebInspector.domAgent.nodeForId(imageId); > + var src = node.getAttribute("src"); > + if (!src) { > + invokeDoneIfNeeded(context); > + return; You could have done this in the injected audit part. > + for (var name in styles.styleAttributes) { > + if (name === "width") { > + widthFound = true; > continue; > - var cssText = (image.style && image.style.cssText) ? image.style.cssText : ""; > - var rules = document.defaultView.getMatchedCSSRules(image, "", true); > - if (!hasWidth(image, cssText, rules) || !hasHeight(image, cssText, rules)) { > - found = true; > - if (urlToNoDimensionCount.hasOwnProperty(image.src)) > - ++urlToNoDimensionCount[image.src]; > - else > - urlToNoDimensionCount[image.src] = 1; > } > + if (name === "height") > + heightFound = true; > + } This part sound confusing to me. Is it equivalent to ("width" in styles.styleAttributes && "height" in styles.styleAttributes)? > + for (var i = styles.matchedCSSRules.length - 1; i >= 0; --i) { > + var style = WebInspector.CSSStyleDeclaration.parseRule(styles.matchedCSSRules[i]).style; > + if (style.getPropertyValue("width") !== "") > + widthFound = true; > + if (style.getPropertyValue("height") !== "") > + heightFound = true; You could add !heightFound || !widthFound into the for check in order to same time here. > + if (context.urlToNoDimensionCount.hasOwnProperty(src)) Why using hasOwnProperty here? > + function receivedImages(imageIds) > + { > + if (!imageIds || !imageIds.length) > + return callback(null); > + var context = {imagesLeft: imageIds.length, urlToNoDimensionCount: {}}; Wrong indent before for (var i = imageIds.length - 1; i >= 0; --i)'s closing tag. > +InjectedScript.getElementsByTagName = function(tagName) > +{ No need to expose this. Just expose pushNodePathToFrontend to the injected audit code. r- is for this.
Created attachment 51305 [details] [PATCH] Comments addressed
Comment on attachment 51305 [details] [PATCH] Comments addressed r+ with nits, please fix before landing. > + var widthFound, heightFound; No need for these to live for so long - just declare them when assigning. > + var nodeId = this.pushNodePathToFrontend(nodes[i]); I'd call this new method 'getNodeId'
Latest comments addressed. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/inspector/front-end/AuditRules.js M WebCore/inspector/front-end/InjectedScript.js Committed r56347