[Webkit-unassigned] [Bug 101910] Web Inspector: line number of script should be search-able in Timeline Panel

[bugzilla-daemon at webkit.org]

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





--- Comment #13 from pdeng6 <pan.deng at intel.com>  2012-11-14 23:46:25 PST ---
(In reply to comment #11)
> (From update of attachment 174122 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=174122&action=review
> 
> > Source/WebCore/inspector/front-end/TimelinePresentationModel.js:888
> > +                    contentHelper._appendTextRow(WebInspector.UIString("Details"), this.detailsNode().textContent);
> 
> I wonder if you should do appendElementRow here in order to not lose the formatting and save on operations.
> 
> > Source/WebCore/inspector/front-end/TimelinePresentationModel.js:930
> > +                detailsContainer.appendChild(document.createTextNode("("));
> 
> I'd bring this back - otherwise, you can't reuse it in the popup above and you add "(" twice in the search matcher.

agree, TimelineRecordListRow.prototype.update and testContentMatching need "()", however popup won't use it, so I write detailsNode() & details()

> 
> I would simply replace 
> return this.data ? this.data["type"] : null;
> with
> return this.data ? createSpan(this.data["type"]) : null;
> in _getRecordDetails
Good point, original _getRecordDetails may return {null|undefined|Node|string}
now, changed to {null|Node}

> 
> I think it is Ok to have it nullable Node. I.e. 

> /**
>  * @return {?Node}
>  */
> _getRecordDetails
> 
> caching could be done via testing for (typeof this._details === "undefined") so that you don't compute null over and over.
Good idea, done!

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