Bug 146688 - Web Inspector: New JavaScript/Probe breakpoint action disappears when clicking continue checkbox
Summary: Web Inspector: New JavaScript/Probe breakpoint action disappears when clickin...
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: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-07-07 12:33 PDT by Matt Baker
Modified: 2015-07-09 01:05 PDT (History)
8 users (show)

See Also:


Attachments
[Patch] Proposed Fix (3.18 KB, patch)
2015-07-07 13:27 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
[Patch] Proposed Fix (3.28 KB, patch)
2015-07-08 22:50 PDT, Matt Baker
no flags Details | Formatted Diff | Diff
[Patch] Proposed Fix (3.28 KB, patch)
2015-07-08 23:36 PDT, Matt Baker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2015-07-07 12:33:40 PDT
* SUMMARY
New JavaScript/Probe breakpoint action disappears when clicking continue checkbox.

Clicking the continue checkbox before entering action text is perfectly reasonable, and it's very frustrating to have to add the action again. The same scenario with a Log Message action doesn't have this problem, the difference being that we allow breakpoints that log an empty message.

* STEPS TO REPRODUCE
1. Open Debugger tab
2. Create a new breakpoint
3. Edit breakpoint, add action
4. Change action to Evaluate JavaScript or Probe Expression
5. Leave action text blank
6. Click continue checkbox
  => New action disappears.
Comment 1 Radar WebKit Bug Importer 2015-07-07 12:34:02 PDT
<rdar://problem/21709406>
Comment 2 Matt Baker 2015-07-07 13:27:47 PDT
Created attachment 256319 [details]
[Patch] Proposed Fix
Comment 3 Timothy Hatcher 2015-07-07 13:33:05 PDT
Comment on attachment 256319 [details]
[Patch] Proposed Fix

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

Good call! This has annoyed me in the past.

> Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:530
> +            return !action.data.length && (action.type === WebInspector.BreakpointAction.Type.Evaluate || action.type === WebInspector.BreakpointAction.Type.Probe);

Maybe action.data.trim().length
Comment 4 Timothy Hatcher 2015-07-07 13:34:23 PDT
Comment on attachment 256319 [details]
[Patch] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:530
>> +            return !action.data.length && (action.type === WebInspector.BreakpointAction.Type.Evaluate || action.type === WebInspector.BreakpointAction.Type.Probe);
> 
> Maybe action.data.trim().length

Also no need for length. !action.data.trim() is the same, since "" is falsey.
Comment 5 Matt Baker 2015-07-07 14:21:03 PDT
Comment on attachment 256319 [details]
[Patch] Proposed Fix

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

>>> Source/WebInspectorUI/UserInterface/Models/Breakpoint.js:530
>>> +            return !action.data.length && (action.type === WebInspector.BreakpointAction.Type.Evaluate || action.type === WebInspector.BreakpointAction.Type.Probe);
>> 
>> Maybe action.data.trim().length
> 
> Also no need for length. !action.data.trim() is the same, since "" is falsey.

BreakPointActionView trims the CodeMirror value before storing in the action.
Comment 6 Matt Baker 2015-07-08 22:50:58 PDT
Created attachment 256460 [details]
[Patch] Proposed Fix
Comment 7 Matt Baker 2015-07-08 23:36:16 PDT
Created attachment 256464 [details]
[Patch] Proposed Fix
Comment 8 Brian Burg 2015-07-09 00:12:20 PDT
Comment on attachment 256464 [details]
[Patch] Proposed Fix

r=me
Comment 9 WebKit Commit Bot 2015-07-09 01:05:01 PDT
Comment on attachment 256464 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 256464

Committed r186589: <http://trac.webkit.org/changeset/186589>
Comment 10 WebKit Commit Bot 2015-07-09 01:05:06 PDT
All reviewed patches have been landed.  Closing bug.