Skip to content

Commit

Permalink
Implement conditional breakpoints with new editor and new bp refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
farshidbeheshti committed May 8, 2013
1 parent 409eade commit b3e0400
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ ConditionEditor.prototype = domplate(Firebug.JSEditor.prototype,
setTimeout(Obj.bindFixed(function()
{
var offset = Dom.getClientOffset(sourceLine);

var bottom = offset.y + sourceLine.offsetHeight;

var y = bottom - this.box.offsetHeight;
if (y < panel.scrollTop)
{
Expand All @@ -73,7 +74,7 @@ ConditionEditor.prototype = domplate(Firebug.JSEditor.prototype,
Css.removeClass(this.box, "upsideDown");
}

this.box.style.top = y + "px";
this.box.style.top = (y - panel.scrollTop) + "px";
Dom.hide(this.box, false);

this.input.focus();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ var BreakpointStore = Obj.extend(Firebug.Module,

// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //

addBreakpoint: function(url, lineNo, type)
addBreakpoint: function(url, lineNo, condition, type)
{
type = type || BP_NORMAL;

Expand All @@ -229,6 +229,17 @@ var BreakpointStore = Obj.extend(Firebug.Module,
return;
}

// Saving the conditional bps physically should be defered until
// the user decides to save the bp.
if (condition != null)
{
var bp = new Breakpoint(url, lineNo, false, type);
bp.condition = condition;

// We just need to find the actual location of bp.
this.dispatch("onAddBreakpoint", [bp]);
return;
}
var bp = this.findBreakpoint(url, lineNo, -1);

// Bail out if exactly the same breakpoint already exists. This is not an error
Expand Down
16 changes: 13 additions & 3 deletions extension/content/firebug/debugger/debuggerTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,19 @@ DebuggerTool.prototype = Obj.extend(new Firebug.EventSource(),
bp.lineNo = currentLine;
}

// Breakpoint is ready on the server side, let's notify all listeners so,
// the UI is properly (and asynchronously) updated everywhere.
self.dispatch("onBreakpointAdded", [self.context, bp]);
if (bp.condition != null)
{
// The actual location of bp is found, So just open the
// condition editor at the corrected line.
self.dispatch("openBreakpointConditionEditor",
[bp.lineNo, bp.condition, bp.params.originLineNo]);
}
else
{
// Breakpoint is ready on the server side, let's notify all listeners so,
// the UI is properly (and asynchronously) updated everywhere.
self.dispatch("onBreakpointAdded", [self.context, bp]);
}

// The info about the original line should not be needed any more.
delete bp.params.originLineNo;
Expand Down
96 changes: 69 additions & 27 deletions extension/content/firebug/debugger/script/scriptPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,11 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
Trace.sysout("scriptPanel.addBreakpoint;", bp);

var url = this.getCurrentURL();
BreakpointStore.addBreakpoint(url, bp.line);
BreakpointStore.addBreakpoint(url, bp.line, bp.condition);

// Enable by default.
BreakpointStore.enableBreakpoint(url, bp.line);
if (bp.condition == null)
BreakpointStore.enableBreakpoint(url, bp.line);
},

removeBreakpoint: function(bp)
Expand Down Expand Up @@ -523,11 +524,11 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
// Conditional Breakpoints

openBreakpointConditionEditor: function(lineIndex, event)
startBreakpointConditionEditor: function(lineIndex, event)
{
Trace.sysout("scriptPanel.openBreakpointConditionEditor; Line: " + lineIndex);
Trace.sysout("scriptPanel.startBreakpointConditionEditor; Line: " + lineIndex);

this.editBreakpointCondition(lineIndex);
this.initializeEditBreakpointCondition(lineIndex);
Events.cancelEvent(event);
},

Expand All @@ -540,21 +541,22 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
Editor.stopEditing(false);
},

editBreakpointCondition: function(lineNo)
initializeEditBreakpointCondition: function(lineNo)
{
// Create helper object for remembering the line and URL. It's used when
// the user right clicks on a line with no breakpoint and picks
// Edit Breakpoint Condition. This should still work and the breakpoint
// should be created automatically if the user provide a condition.
var tempBp = {
lineNo: lineNo,
href: this.getCurrentURL(),
condition: "",
};

var url = this.getCurrentURL(), editor = this.getEditor();
// The breakpoint doesn't have to exist.
var bp = BreakpointStore.findBreakpoint(this.getCurrentURL(), lineNo);
var condition = bp ? bp.condition : tempBp.condition;
var bp = BreakpointStore.findBreakpoint(url, lineNo);

if (bp)
{
// Reference to the edited breakpoint.
editor.breakpoint = bp;

// if there is alreay a bp, the line is executable, so we just need to
// open the editor.
this.openBreakpointConditionEditor(lineNo, bp.condition);
return;
}

// xxxHonza: displaying BP conditions in the Watch panel is not supported yet.
/*if (condition)
Expand All @@ -564,21 +566,60 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
watchPanel.rebuild();
}*/

// Reference to the edited breakpoint.
var editor = this.getEditor();
editor.breakpoint = bp ? bp : tempBp;
this.scriptView.initializeBreakpoint(lineNo, condition);
// Create helper object for remembering the line and URL. It's used when
// the user right clicks on a line with no breakpoint and picks
// Edit Breakpoint Condition. This should still work and the breakpoint
// should be created automatically if the user provide a condition.
var tempBp = {
lineNo: lineNo,
href: url,
condition: "",
};

editor.breakpoint = tempBp;
this.scriptView.initializeBreakpoint(lineNo, tempBp.condition);
},

startEditingCondition: function(lineNo, condition)
openBreakpointConditionEditor: function(lineNo, condition, originalLineNo)
{
var target = this.scriptView.getAnnotationTarget(lineNo);
var bp = BreakpointStore.findBreakpoint(this.getCurrentURL(), lineNo);
var target = null;

// If the line the user clicked wasn't a executable line, it's need
if (originalLineNo)
this.scriptView.removeBreakpoint({lineNo: originalLineNo});


if (!bp)
{
// If a bp didn't exist at the line, loading icon is showing
// and it's need to remove it, otherwise the loading icon
// isn't shown if the uset wanted to set condition on a existed
// bp (See initializeEditBreakpointCondition()).
this.scriptView.removeBreakpoint({lineNo: lineNo});
}
else
{
// There is already a bp at the line, so get the element(target)
// of bp icon. we should also verify if the bp is a conditional
// bp, if so, load the expression into the editor.
target = this.scriptView.getGutterMarkerTarget(lineNo);
condition = bp.condition;
}

if (!target)
return;
{
this.scriptView.addBreakpoint({lineNo: lineNo});
target = this.scriptView.getGutterMarkerTarget(lineNo);
}

var conditionEditor = this.getEditor();
conditionEditor.breakpoint.lineNo = lineNo;

// As Editor scrolls(not panel itself) with long scripts, we need to set
// scrollTop manually to show the editor properly(at the right y coord).
this.scrollTop = this.scriptView.getScrollInfo().top;

Firebug.Editor.startEditing(target, condition, null, null, this);
},

Expand All @@ -598,18 +639,19 @@ ScriptPanel.prototype = Obj.extend(BasePanel,
Trace.sysout("scriptPanel.onSetBreakpointCondition; " + value, bp);

var availableBp = BreakpointStore.findBreakpoint(bp.href, bp.lineNo);

if (!cancel)
{
if (!availableBp)
this.setBreakpoint(bp);
this.addBreakpoint({line: bp.lineNo});

value = value ? value : null;
BreakpointStore.setBreakpointCondition(bp.href, bp.lineNo, value);
}
else
{
if (!availableBp)
BreakpointStore.removeBreakpoint(bp.href, bp.lineNo);
this.scriptView.removeBreakpoint({lineNo: bp.lineNo});
}
},

Expand Down
65 changes: 31 additions & 34 deletions extension/content/firebug/debugger/script/scriptView.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
this.onMouseMoveListener = this.onMouseMove.bind(this);
this.onMouseOutListener = this.onMouseOut.bind(this);
this.onGutterClickListener = this.onGutterClick.bind(this);
this.OnMouseUpListener = this.onEditorMouseUp.bind(this);

// Initialize source editor.
this.editor = new SourceEditor();
Expand All @@ -98,9 +99,10 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
this.editor.addEventListener(SourceEditor.Events.gutterClick,
this.onGutterClickListener);

/*// Hook view body mouse up (for breakpoint condition editor).
this.editor._view._handleBodyMouseUp = this.bodyMouseUp.bind(this);
*/
// Hook view body mouse up (for breakpoint condition editor).
this.editor.addEventListener(SourceEditor.Events.mouseUp,
this.OnMouseUpListener);

// Focus so, keyboard works as expected.
this.editor.focus();

Expand Down Expand Up @@ -371,18 +373,19 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
}
},

initializeBreakpoint: function(lineIndex)
initializeBreakpoint: function(lineIndex, condition)
{
var bpWaiting = this.editor.getGutterElement().ownerDocument.createElement("div");
bpWaiting.className = "breakpointLoading";


this.editor.setGutterMarker(SourceEditor.Gutters.breakpoints,
lineIndex, bpWaiting);

// Simulate editor event sent when the user creates a breakpoint by
// clicking on the breakpoint ruler.
this.onBreakpointChange({
added: [{ line: lineIndex}],
added: [{ line: lineIndex, condition: condition}],
removed: []
});
},
Expand All @@ -392,6 +395,7 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
var bpMarker = this.editor.getGutterMarker(SourceEditor.Gutters.breakpoints,
bp.lineNo);

bpMarker.className = "breakpoint";
if (bp.disabled)
{
bpMarker.className += " disabled";
Expand Down Expand Up @@ -449,6 +453,11 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
this.highlightLine(line);
},

getScrollInfo: function()
{
return this.editor.getScrollInfo();
},

highlightLine: function(lineIndex)
{
Trace.sysout("scriptView.highlightLine; " + lineIndex);
Expand Down Expand Up @@ -480,28 +489,21 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
Trace.sysout("scriptView.gutterClick; " + event);

if (event.lineNo != null)
this.toggleBreakpoint(event.lineNo);
{
// We are interested in right-click events to open
// breakpoint condition editor.
if (Events.isRightClick(event.rawEvent))
this.dispatch("startBreakpointConditionEditor", [event.lineNo, event.rawEvent]);
else
this.toggleBreakpoint(event.lineNo);
}
},

bodyMouseUp: function(event)
onEditorMouseUp: function (event)
{
Trace.sysout("scripView.bodyMouseUp;", event);
Trace.sysout("scripView.onEditorMouseUp;", event);

this.dispatch("onEditorMouseUp", [event]);

// We are only interested in right-click events...
if (!Events.isRightClick(event))
return;

// ... on the breakpoint-column (to show the breakpoint condition editor).
var target = event.target;
var ruler = Dom.getAncestorByClass(target, "ruler");
if (!Css.hasClass(ruler, "annotations") && !Css.hasClass(ruler, "lines"))
return;

// The breakpoint condition editor is about to be opened.
var lineIndex = this.getLineIndex(target);
this.dispatch("openBreakpointConditionEditor", [lineIndex, event]);
},

getLineIndex: function(target)
Expand Down Expand Up @@ -531,20 +533,15 @@ ScriptView.prototype = Obj.extend(new Firebug.EventSource(),
* Returns related annotation target (an element) associated with the given line.
* @param {Object} line Given line number. Line numbers are zero-based.
*/
getAnnotationTarget: function(line)
getGutterMarkerTarget: function(line, gutter)
{
// This method is using Orion's private API.
var viewLeftRuler = this.editor._view._leftDiv;
var annotationsRuler = viewLeftRuler.querySelector(".ruler.annotations");

// Search through the annotations for the one associated with the given
// line number.
var length = annotationsRuler.children.length;
for (var i=0; i<length; i++)
gutter = gutter || SourceEditor.Gutters.breakpoints;

var marker = this.editor.getGutterMarker(gutter, line);
if (marker)
{
var annotation = annotationsRuler.children[i];
if (annotation.lineIndex == line)
return annotation;

return marker;
}

return null;
Expand Down
10 changes: 8 additions & 2 deletions extension/content/firebug/editor/sourceEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ SourceEditor.DefaultConfig =
fixedGutter: false,
readOnly: true,
showCursorWhenSelecting: true,
undoDepth: 200,
undoDepth: 200

// xxxHonza: this is weird, when this props is set the editor is displayed twice.
// There is one-line editor created at the bottom of the Script panel.
Expand All @@ -84,7 +84,8 @@ SourceEditor.Events =
contextMenu: "contextmenu",
mouseMove: "mousemove",
mouseOut: "mouseout",
mouseOver: "mouseover"
mouseOver: "mouseover",
mouseUp: "mouseup"
};

// ********************************************************************************************* //
Expand Down Expand Up @@ -397,6 +398,11 @@ SourceEditor.prototype =
}
},

getScrollInfo: function()
{
return this.editorObject.getScrollInfo();
},

getTopIndex: function()
{
var rect = this.editorObject.getWrapperElement().getBoundingClientRect();
Expand Down

0 comments on commit b3e0400

Please sign in to comment.