Bug 116191 - Web Inspector: InspectorInstrumentation::willEvaluateScript should include column number
Summary: Web Inspector: InspectorInstrumentation::willEvaluateScript should include co...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-05-15 15:55 PDT by Joseph Pecoraro
Modified: 2019-01-22 20:51 PST (History)
6 users (show)

See Also:


Attachments
Patch (20.76 KB, patch)
2019-01-09 10:25 PST, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (31.76 KB, patch)
2019-01-22 19:14 PST, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 Brian Burg 2016-08-03 11:52:58 PDT
This certainly works now.
Comment 4 Brian 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.