Closed Bug 497177 Opened 15 years ago Closed 15 years ago

Firebug breakpoints are not working.

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: cbartley, Unassigned)

References

Details

(Keywords: regression, testcase, Whiteboard: [firebug-p1])

Attachments

(2 files)

      No description provided.
What version of Firebug?  Does this also happen on 1.9.1?
Flags: blocking1.9.1?
Sorry about that.  I just written the title and accidentally submitted it
before I was done.  I suspect an accidental mouse pad gesture cause I swear my
fingers were no where near the return key.

More information forthcoming shortly.
I'm seeing this on Mozilla trunk at 29011:5d1bf7e2c8dd.  I was not seeing it before updating to this revision, and my last update was yesterday sometime.
My questions in comment #1 stand, I think.

Getting a tracemonkey regression range on this would be great, by which I think I mean "vital".
I've reproed this bug on Firebug1.4 HEAD and Firebug 1.4a31.
Attached file Simple test case
Repro Steps

   1. Install the latest Firebug 1.4 alpha, for example firebug-1.4.0a31.xpi.
   2. Open this page in Firefox.
   3. Open Firebug and make sure it's completely enabled for the page.
   4. Tab to the Script panel.
   5. Set a breakpoint on the first call to yada() in OnClick()
   6. Click the button

Expected Behavior

    * Firefox breaks at the breakpoint and Firebug enters single step mode

Actual Behavior

    * Firefox does not break at the breakpoint.
The bug does not reproduce on 1.9.1 tip.  It breaks into the debugger,and you can single step.  The browser usually crashes shortly after that.  I think we may already have a bug open for that particular behavior, I'll check.
Whiteboard: [firebug-p1]
OK, then not a blocker. Thanks,
Flags: blocking1.9.1?
Whiteboard: [firebug-p1]
wondering if this is additional fallout from bug 494235?
hm, no, that's probably too early. Doing a little regression hunting...
(In reply to comment #7)
> The bug does not reproduce on 1.9.1 tip.  It breaks into the debugger,and you
> can single step.  The browser usually crashes shortly after that.  I think we
> may already have a bug open for that particular behavior, I'll check.

Opened bug 497212 for 1.9.1.  The test cases are very similar but the erroneous behavior is very different, so I don't know how closely the bugs are related.
in your testcase, do you reload the page before clicking "Click me"? Some of Firebug's debugging can be a bit wonky if it doesn't have a full view of all the scripts that were compiled at debug time.
I should note test passes if I reload in a build with c3f03e38cdb4. Does not pass if I don't reload, so I'm not sure I'm testing this properly.
(In reply to comment #13)
> I should note test passes if I reload in a build with c3f03e38cdb4. Does not
> pass if I don't reload, so I'm not sure I'm testing this properly.

I'm still seeing this bug on m-c tip and Firebug1.4 HEAD.  Reloading doesn't seem to make any difference.
The regression happens in rev 29004 on trunk.

Pass: 29003 http://hg.mozilla.org/mozilla-central/rev/66a40d5fda11
Fail: 29004 http://hg.mozilla.org/mozilla-central/rev/39f1a74e500b
29003:66a40d5fda11 was a tracemonkey merge...
(In reply to comment #13)
> I should note test passes if I reload in a build with c3f03e38cdb4. Does not
> pass if I don't reload, so I'm not sure I'm testing this properly.

I can only reproduce this bug in trunk.  If I try to reproduce it on the 1.9.1 branch I see the behavior where it is sometimes necessary to reload the page before setting the breakpoint, but otherwise it works.
Blocks: 497119
No longer blocks: 497119
As discussed with Curtis via IRC, I can reproduce this.

It consistently happens when the "Watch" panel is focused. When the "Breakpoints" or "Stack" panel is open I will hit the breakpoint.
That sure does sound like a Firebug-side bug?  I have a hard time understanding how the JS engine's breakpoint behaviour would depend on the focus state of the FB UI.
(In reply to comment #20)
> That sure does sound like a Firebug-side bug?  
 
I agree that this sounds like Firebug to me.
(In reply to comment #20)
> That sure does sound like a Firebug-side bug?  I have a hard time understanding
> how the JS engine's breakpoint behaviour would depend on the focus state of the
> FB UI.

I find that hard to understand as well.  Nevertheless that regression range I did yesterday looks mighty suspicious.

Anyway, I'll investigate further on the Firebug side and see what I can learn.
On IRC curtis explained that this is 1.9.2 only, works on 1.9.1
Blocks: 491553
(In reply to comment #23)
> On IRC curtis explained that this is 1.9.2 only, works on 1.9.1

To summarize, I have two reasons for suspecting that this is not a Firebug bug.

1. The bug only repros in trunk, not in branch.
2. The bug only appeared in trunk after changeset 29003:66a40d5fda11.

It turns out 29003:66a40d5fda11 is a tracemonkey merge, which is, as I said above, awfully suspicious.
OK, could be -- find the regressing changeset in tracemonkey?  That'll probably tell the tale.
From the Firebug trace console, I see the following error:

Resuming debugger: error during debugging loop: TypeError: parent is undefined parent is undefined

This particular error appears when the breakpoint is ignored (Watch panel is visible) and does not appear when the breakpoint is hit (Breakpoints panel is visible).
If you click on the trace console line and select object you should get more info on the exception; also the stack
(In reply to comment #27)
> If you click on the trace console line and select object you should get more
> info on the exception; also the stack

Hmm, I think there's a bug in the trace console -- the extra information seemed to be MIA.  I had to scroll way over to the right to find it.

Anyway, the call stack for this error is:

chrome://firebug/content/domplate.js (435) __path__((void 0),0,-1,0,1,0)[...]
chrome://firebug/content/domplate.js (441) (0,0,anonymous,[object Array])[...]
chrome://firebug/content/domplate.js (417) __loop__([object Array],(function (i0, l0, d0, d1) {node = __path__(root, o, -1, 0 + l0 + 0, 1, 0);e0 = __link__(node, d0, d1);return 1;}))[...]
chrome://firebug/content/domplate.js (441) ([object HTMLTableRowElement],(void 0),0,[object Array])[...]
chrome://firebug/content/domplate.js (917) ([object Object],[object HTMLTableRowElement])[...]
chrome://firebug/content/dom.js (300) ([object Array],false)[...]
chrome://firebug/content/dom.js (1215) ([object XPCWrappedNative_NoHelper])[...]
chrome://firebug/content/firebug.js (2098) ([object XPCWrappedNative_NoHelper])[...]
chrome://firebug/content/chrome.js (875) ([object XPCWrappedNative_NoHelper],[object Object])[...]
chrome://firebug/content/firebug.js (2101) ([object XPCWrappedNative_NoHelper],true)[...]
chrome://firebug/content/chrome.js (469) ([object XPCWrappedNative_NoHelper],"script",null,true)[...]
chrome://firebug/content/debugger.js (598) ([object Object])[...]
chrome://firebug/content/lib.js (94) ()[...]
file:///Users/cbartley/Dev/firebug/mainline-firebug1.4/components/firebug-service.js (401) ()[...]
OK, tracemonkey is off the hook here.  The problem was introduced by r29004, http://hg.mozilla.org/mozilla-central/rev/39f1a74e500b: Bug 468708 - namespaceURI for HTML elements should be http://www.w3.org/1999/xhtml (also make localName return in lower case) r=sicking, sr=peterv, a=beltzner.

This looks like primarily DOM changes, which probably explains the Watch panel's wonky layout.  I'm not sure why it would interfere with breakpoints.  I'll investigate that next.
(In reply to comment #29)
> This looks like primarily DOM changes, which probably explains the Watch
> panel's wonky layout.  I'm not sure why it would interfere with breakpoints. 
> I'll investigate that next.

The error in comment 28 aborts the breakpoint I believe, see debugger.js line 606
It's time to get Firebug working on FF 3.6.
Whiteboard: [firebug-p1]
So if bug 468708 is the problem, and it is a standards-compliance fix as it appears to be, is it possible that FB needs to adapt to the new namespaceURI and/or the new localName case?  I'm not immediately sure why that would cause a variable to not be undefined, though, but maybe Jonas or Henri do?

Curtis or Rob: if you back out that changeset locally, does it fix things?
In firebug's source repo, I see suspicious code in domplate.js (string literal "TR" should be "tr", multiple occurrences).
Can anyone point to a developer doc on this change? I'm looking for what the rules are and which version of Firefox enforces which set.
DOM Level 1 APIs work as before. Since 3.6a, DOM Level 2 APIs are now unified between HTML and XHTML. More precisely, the DOM APIs work the same with HTML and XHTML trees except for the differences documented in
http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#apis-in-html-documents

This means that there's no longer need for the duplication in Firebug's lib.js in nonEditableTags, innerEditableTags and invisibleTags if you want Firefox 3.6 or later only code.

The HTMLness vs. XMLness is now on a per-Document basis.

In Firefox 3.5 and earlier, localName returned in upper case and namespaceURI was null. The HTMLness vs. XMLness was on a per-Element basis.

You shouldn't call createElementNS(null, "div") to create a div element, since that's not cross-version (or for Web content cross-browser) compatible. You should either call createElement("div") or createElementNS("http://www.w3.org/1999/xhtml", "div").
(In reply to comment #33)
> In firebug's source repo, I see suspicious code in domplate.js (string literal
> "TR" should be "tr", multiple occurrences).

Thanks, but it this comment that I don't understand. (We don't have any calls like createElementNS(null, "div")).
localNames now trade in lowercase strings, not uppercase strings, so if you're using "TR" instead of "tr" and comparing them to localNames, you will now lose with FF 3.6+
(In reply to comment #37)
> localNames now trade in lowercase strings, not uppercase strings, so if you're
> using "TR" instead of "tr" and comparing them to localNames, you will now lose
> with FF 3.6+

Thanks, but a quick look suggests that this is a red-herring. Every comparison using localName seems to convert case. (Unless the "TR" values are essentially incorrect now.)

I'll look into this on the Firebug side.
Added https://developer.mozilla.org/en/Updating_extensions_for_Firefox_3.6 and updated the DevMo DOM reference accordingly.
The attached test case (comment 6) on Firebug 1.5a21 with 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090820 Minefield/3.7a1pre

does not fail the test (breakpoint hits) and the watch panel is not wonky.

So the next step is for someone to repro this with 1.5a21 on FF 3.7, then open the tracing console in Firebug and run with ERRORS and UI_LOOP set. 

If the bug is still there, my guess at this point would be a MacOS exception that causes the Watch panel display code to abort (wonky) and this gets caught by the debugger who then aborts the debug session (no stop on breakpoint).
(In reply to comment #38)
> (In reply to comment #37)
> > localNames now trade in lowercase strings, not uppercase strings, so if you're
> > using "TR" instead of "tr" and comparing them to localNames, you will now lose
> > with FF 3.6+
> 
> Thanks, but a quick look suggests that this is a red-herring. Every comparison
> using localName seems to convert case. (Unless the "TR" values are essentially
> incorrect now.)
> 
> I'll look into this on the Firebug side.

Sorry, Google showed me a cached old copy instead of the current one, even though I initiated the search from the project site and not on the code search site.
I  ran the test in comment #6 and it passes on
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091005 Minefield/3.7a1pre

Note that nightly builds show bug 520421 so the execution line will not be highlighted until you bring your mouse over the line numbers.

Shall we move this back to unconfirmed or resolved ?
done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: