Bug 122151 - Web Inspector: evaluate and show a popover for selected text in JS source when paused
Summary: Web Inspector: evaluate and show a popover for selected text in JS source whe...
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: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-10-01 02:56 PDT by Antoine Quint
Modified: 2013-10-01 12:54 PDT (History)
5 users (show)

See Also:

Attachments
Patch (5.10 KB, patch)
2013-10-01 03:00 PDT, Antoine Quint 		no flags Details | Formatted Diff | Diff
Patch for landing (5.05 KB, patch)
2013-10-01 12:10 PDT, Antoine Quint 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2013-10-01 02:56:10 PDT 
<rdar://problem/14043399>
Comment 1 Antoine Quint 2013-10-01 02:56:29 PDT 
Expanding the popover on hover, we should show the popover for the expression of the selected text. This gives the user more control over what to evaluate if the expression is hard to figure out by hover alone.
Comment 2 Antoine Quint 2013-10-01 03:00:56 PDT 
Created attachment 213068 [details]
Patch
Comment 3 Joseph Pecoraro 2013-10-01 10:10:22 PDT 
Comment on attachment 213068 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/CodeMirrorTokenTrackingController.js:168
> +            if (highlightedRange.from.line === range.start.line &&
> +                highlightedRange.from.ch === range.start.ch &&
> +                highlightedRange.to.line === range.end.line &&
> +                highlightedRange.to.ch === range.end.ch)

This could use WebInspector.compareCodeMirrorPositions:

    if (WebInspector.compareCodeMirrorPositions(highlightedRange.from, range.start) === 0 &&
        WebInspector.compareCodeMirrorPositions(highlightedRange.to, range.end) === 0)

But what you have is just fine!
Comment 4 Antoine Quint 2013-10-01 12:10:07 PDT 
(In reply to comment #3)
> (From update of attachment 213068 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=213068&action=review
> 
> r=me
> 
> > Source/WebInspectorUI/UserInterface/CodeMirrorTokenTrackingController.js:168
> > +            if (highlightedRange.from.line === range.start.line &&
> > +                highlightedRange.from.ch === range.start.ch &&
> > +                highlightedRange.to.line === range.end.line &&
> > +                highlightedRange.to.ch === range.end.ch)
> 
> This could use WebInspector.compareCodeMirrorPositions:
> 
>     if (WebInspector.compareCodeMirrorPositions(highlightedRange.from, range.start) === 0 &&
>         WebInspector.compareCodeMirrorPositions(highlightedRange.to, range.end) === 0)
> 
> But what you have is just fine!

Much better, I'll change that in the commit.
Comment 5 Antoine Quint 2013-10-01 12:10:21 PDT 
Created attachment 213107 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2013-10-01 12:54:05 PDT 
Comment on attachment 213107 [details]
Patch for landing

Clearing flags on attachment: 213107

Committed r156728: <http://trac.webkit.org/changeset/156728>
Comment 7 WebKit Commit Bot 2013-10-01 12:54:07 PDT 
All reviewed patches have been landed.  Closing bug.