Do not use "this" within non-prototypes, get rid of WebInspector dependencies
Created attachment 104669 [details] Patch
Created attachment 104673 [details] Patch
Comment on attachment 104673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104673&action=review > Source/WebCore/inspector/front-end/ConsoleView.js:999 > + formatters.s = formatters.f = formatters.i = formatters.d = valueFormatter; Split to multiple lines or use a for? > Source/WebCore/inspector/front-end/DOMAgent.js:249 > + var currentNode = node.parentNode; > + while (currentNode) { > + if (this === currentNode) > + return true; > + currentNode = currentNode.parentNode; > + } nit: this is better written as a for loop (I know you're just moving it around, hence just a nit) > Source/WebCore/inspector/front-end/DOMAgent.js:255 > + return descendant !== null && descendant.isAncestor(this); return !!descendant && descendant.isAncestor(this); > Source/WebCore/inspector/front-end/DOMAgent.js:264 > + if (!this.nodeValue.length) > + return true; > + return this.nodeValue.match(/^[\s\xA0]+$/); nit: do we need an explicit optimization here? also, why do we use match() instead of test()? This would be more readable as /^\s*$/.test(this.nodeValue); > Source/WebCore/inspector/front-end/ElementsPanel.js:482 > + if (!updateBreadcrumbs && (this.selectedDOMNode() === parent || this.selectedDOMNode().isAncestor(parent))) This seems to have slightly different semantics to the old code. > Source/WebCore/inspector/front-end/utilities.js:481 > while (currentNode) { > - if (ancestor === currentNode) > + if (this === currentNode) > return true; > currentNode = currentNode.parentNode; > } nit: for? > Source/WebCore/inspector/front-end/utilities.js:487 > + return descendant !== null && descendant.isAncestor(this); !!descendant && ...
Comment on attachment 104673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104673&action=review >> Source/WebCore/inspector/front-end/ConsoleView.js:999 >> + formatters.s = formatters.f = formatters.i = formatters.d = valueFormatter; > > Split to multiple lines or use a for? Done. >> Source/WebCore/inspector/front-end/DOMAgent.js:249 >> + } > > nit: this is better written as a for loop (I know you're just moving it around, hence just a nit) I don't want to change anything... >> Source/WebCore/inspector/front-end/DOMAgent.js:255 >> + return descendant !== null && descendant.isAncestor(this); > > return !!descendant && descendant.isAncestor(this); Done. >> Source/WebCore/inspector/front-end/utilities.js:481 >> } > > nit: for? Did not want to change much >> Source/WebCore/inspector/front-end/utilities.js:487 >> + return descendant !== null && descendant.isAncestor(this); > > !!descendant && ... Done.
Created attachment 104678 [details] Patch
Comment on attachment 104678 [details] Patch LGTM
Comment on attachment 104678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104678&action=review > Source/WebCore/inspector/front-end/inspector.js:1720 > +Number.secondsToString = function(seconds, higherResolution) Might as well fix these two methods up with some nice JS doc comments now.
Committed r93584: <http://trac.webkit.org/changeset/93584>