[webkit-reviews] review denied: [Bug 101910] Web Inspector: line number of script should be search-able in Timeline Panel : [Attachment 173890] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied pdeng6 <pan.deng at intel.com>'s
request for review:
Bug 101910: Web Inspector: line number of script should be search-able in
Timeline Panel
https://bugs.webkit.org/show_bug.cgi?id=101910

Attachment 173890: Patch
https://bugs.webkit.org/attachment.cgi?id=173890&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=173890&action=review


> Source/WebCore/inspector/front-end/TimelinePresentationModel.js:930
> +	   if (details && details.hasOwnProperty('textContent'))

You should check for (details instanceof Node) and then use textContent.

But as Eugene mentioned, the fact that details() returns either string or Node
is the root of the problem. I'd rename details() to detailsNode() and create a
span with text in case of text content there. Then I would use
detailsNode.textContent in testContentMatching and removed the similar type
check from WebInspector.TimelineRecordListRow.prototype.update + would also use
appendElementRow in _generatePopupContentWithImagePreview.