1163540 - Can't set breakpoints in tabbrowser.xml

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[:Gijs (he/him)]

With current fx-team tip:

1) open Nightly
2) open browser debugger
3) find tabbrowser.xml in the debugger
4) try to set a breakpoint in _beginRemoveTab or _endRemoveTab

ER:
get breakpoint

AR:
get breakpoint at line 5004 or thereabouts:

      <handler event="transitionend"><![CDATA[
here ==>  if (event.propertyName != "max-width")

Comment 1

[Eddy Bruel [:ejpbruel]]

This is probably fallout from bug 1158498.

Comment 2

[James Long (:jlongster)]

I bet it is from that bug. I suspect that the `element` property does not exist here XUL is weird (and this is unusual inline code). Our check to see if it's inline does not work with this, so we should add back the special case for the ".xul" extension. Eddy knows what I'm talking about...

Comment 3

[James Long (:jlongster)]

We should fix this soon

Comment 5

[James Long (:jlongster)]

Attachment: 1163540.patch

Can you see if this patches fix the problem for you?

I will look into in more depth soon, but if you want a quick fix I think this should do it.

Comment 6

[:Gijs (he/him)]

Comment on attachment 8608796 [details] [diff] [review]
1163540.patch

Review of attachment 8608796 [details] [diff] [review]:
-----------------------------------------------------------------

This wfm in terms of making things work again, though I do still see stuff like:


chrome://browser/content/tabbrowser.xml threw an exception: SyntaxError: expected expression, got '<'
Stack: Parser.prototype.get@resource:///modules/devtools/Parser.jsm:65:21
FilterView.prototype._doSearch@chrome://browser/content/devtools/debugger/filter-view.js:442:20
FilterView.prototype._doTokenSearch@chrome://browser/content/devtools/debugger/filter-view.js:483:5
FilterView.prototype._addCommands/<.tokenSearchCommand@chrome://browser/content/devtools/debugger/filter-view.js:113:33
Line: 65, column: 20

in my console.

Comment 7

[James Long (:jlongster)]

Eddy I'll follow-up with this. Glad that this patch fixes it. We need to get a test for this, I'll see what I can do

Comment 8

[James Long (:jlongster)]

We need an XBL file to add as a test. Something we can load up as a chrome:// page and set a breakpoint in. If you get me that, I will add it and add some basic breakpoint tests on it.

Comment 9

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to :Gijs Kruitbosch from comment #6)
> chrome://browser/content/tabbrowser.xml threw an exception: SyntaxError:
> expected expression, got '<'
> Stack: Parser.prototype.get@resource:///modules/devtools/Parser.jsm:65:21
> FilterView.prototype._doSearch@chrome://browser/content/devtools/debugger/
> filter-view.js:442:20
> FilterView.prototype._doTokenSearch@chrome://browser/content/devtools/
> debugger/filter-view.js:483:5
> FilterView.prototype._addCommands/<.tokenSearchCommand@chrome://browser/
> content/devtools/debugger/filter-view.js:113:33
> Line: 65, column: 20
> 
> in my console.

Yet another reason why we need to stop the pretend-all-inline-html-sources-are-one-source madness: it breaks the smart search for function definitions and all that stuff that relies on parsing the source as JS (but we should still make it fail gracefully, bug 1167303).

Comment 10

[:Gijs (he/him)]

(I'm sorry for the delays, I had a long weekend and got back to a massive queue of stuff. I hope to get to providing a testcase here tomorrow.)

Comment 11

[:Gijs (he/him)]

Attachment: testcase for debugging XUL+XBL

Comment 12

[:Gijs (he/him)]

This wfm in that it loads and the binding attaches and Does Stuff. I've purposefully added most of the things I've seen in XBL (constructor, destructor, cdata/no-cdata things, handlers, methods, properties, fields). I just realized I've forgotten to add the observer method "observe" that handles callbacks from the pref observer I'm adding. Left as an exercise for the reader, I guess. :-)

(you will want to remove the bogus yield new Promise(() => false); from the end of the test, and handle opening/closing tabs the way you usually would, of course... but I don't know much about debugger tests, so I've done what I would have done had it been a browser test)

Comment 13

[:Gijs (he/him)]

James, is there anything else I can do to help this along? This is very frustrating. :-)

Comment 14

[James Long (:jlongster)]

I can push the fix through, but I'll have to add a test later for this. I don't have time right now to work on the test (stuff like that usually takes about a day or so to completely get right).

Comment 15

[James Long (:jlongster)]

Attachment: 1163540.patch

rebased

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7f2c34d411de

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7f2c34d411de