Bug 116191

Summary:

Web Inspector: InspectorInstrumentation::willEvaluateScript should include column number

Product:

WebKit

Reporter:

Joseph Pecoraro <joepeck>

Component:

Web Inspector

Assignee:

Devin Rousso <hi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none

Description Joseph Pecoraro 2013-05-15 15:55:05 PDT

This will likely be necessary for jump to location to work as expected with minified code in the Scripts timeline.

Comment 1 Radar WebKit Bug Importer 2013-05-15 15:55:18 PDT

<rdar://problem/13905910>

Comment 2 Timothy Hatcher 2014-01-10 15:37:16 PST

Moving to the right component.

Comment 3 BJ Burg 2016-08-03 11:52:58 PDT

This certainly works now.

Comment 4 BJ Burg 2016-08-03 11:53:48 PDT

Oh, just kidding. But we only use these instrumentations in the overview afaik.

Comment 5 Devin Rousso 2019-01-09 10:25:12 PST

Created attachment 358715 [details]
Patch

EWS check

Comment 6 EWS Watchlist 2019-01-09 10:27:15 PST

Attachment 358715 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment 7 Joseph Pecoraro 2019-01-09 11:03:58 PST

Comment on attachment 358715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358715&action=review

Nice! r=me.

We don't have any tests on Timeline events right now, we should tackle that at some point.

> Source/WebCore/ChangeLog:9
> +        No new tests (OOPS!).

Seems we can have a test.

> Source/WebCore/inspector/TimelineRecordFactory.cpp:107
> -Ref<JSON::Object> TimelineRecordFactory::createEvaluateScriptData(const String& url, double lineNumber)
> +Ref<JSON::Object> TimelineRecordFactory::createEvaluateScriptData(const String& url, double lineNumber, double columnNumber)

Hmm, why are these doubles? Maybe we can convert them to ints up the chain. JSON::Object::setInteger implicitly converts it to an int anyways.

Comment 8 Devin Rousso 2019-01-22 19:14:58 PST

Created attachment 359835 [details]
Patch

Comment 9 WebKit Commit Bot 2019-01-22 20:51:51 PST

Comment on attachment 359835 [details]
Patch

Clearing flags on attachment: 359835

Committed r240323: <https://trac.webkit.org/changeset/240323>

Comment 10 WebKit Commit Bot 2019-01-22 20:51:53 PST

All reviewed patches have been landed.  Closing bug.