Bug 138839 - Web Inspector: Debugger should not mutate variable when hovering mouse over ++n expression
Summary: Web Inspector: Debugger should not mutate variable when hovering mouse over +...
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: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-11-18 11:32 PST by Joseph Pecoraro
Modified: 2014-11-19 10:34 PST (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (2.29 KB, patch)
2014-11-18 11:36 PST, Joseph Pecoraro
timothy: review+
timothy: commit-queue-
Details | Formatted Diff | Diff
[PATCH] Proposed Fix (2.42 KB, patch)
2014-11-18 14:01 PST, Joseph Pecoraro
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 2014-11-18 11:32:54 PST
* SUMMARY
When debugging javascript paused in the debugger, hovering over a line of the form ++n actually mutates the value of n. n++ does not have this behavior.

* TEST
<script type="text/javascript">
function test() {
    debugger;
    var n = 0;
    console.log(n);
    // Hover over next line. n shouldn't change
    n++;
    console.log(n);
    // Hover over next line. n changes but shouldn't
    ++n;
    console.log(n);
}
document.onload = test();
</script>

* STEPS TO REPRODUCE
1. Inspect test case
2. Reload or type test() into the console
3. Test hovering over n++ and see it doesn't mutate
4. Test hovering over ++n and see that it mutates n every time you hover over it
Comment 1 Radar WebKit Bug Importer 2014-11-18 11:33:10 PST
<rdar://problem/19018280>
Comment 2 Joseph Pecoraro 2014-11-18 11:36:57 PST
Created attachment 241796 [details]
[PATCH] Proposed Fix
Comment 3 Joseph Pecoraro 2014-11-18 11:37:53 PST
<rdar://problem/18995374>
Comment 4 Timothy Hatcher 2014-11-18 11:51:02 PST
Comment on attachment 241796 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorTokenTrackingController.js:490
> +            if (isExpression && token.string === "++")

What about --?
Comment 5 Joseph Pecoraro 2014-11-18 12:03:50 PST
(In reply to comment #4)
> Comment on attachment 241796 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241796&action=review
> 
> > Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorTokenTrackingController.js:490
> > +            if (isExpression && token.string === "++")
> 
> What about --?

Doh! Tim also caught other operators, like:

    n+=n;

I'll put up another patch.
Comment 6 Joseph Pecoraro 2014-11-18 14:01:06 PST
Created attachment 241810 [details]
[PATCH] Proposed Fix
Comment 7 Timothy Hatcher 2014-11-19 09:54:02 PST
Comment on attachment 241810 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorTokenTrackingController.js:492
> +            // Disallow operators. We want the hovered expression to be just a single operand.
> +            // Also, some operators can modify values, such as pre-increment and assignment operators.
> +            if (isExpression && token.type.contains("operator"))
> +                break;

Nice!
Comment 8 WebKit Commit Bot 2014-11-19 10:34:15 PST
Comment on attachment 241810 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 241810

Committed r176325: <http://trac.webkit.org/changeset/176325>
Comment 9 WebKit Commit Bot 2014-11-19 10:34:18 PST
All reviewed patches have been landed.  Closing bug.