Bug 122151

Summary:

Web Inspector: evaluate and show a popover for selected text in JS source when paused

Product:

WebKit

Reporter:

Antoine Quint <graouts>

Component:

Web Inspector

Assignee:

Antoine Quint <graouts>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, graouts, joepeck, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch for landing	none

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.