Closed Bug 771490 Opened 12 years ago Closed 4 years ago

The first line is always referenced in the stack trace for HTML / XUL documents

Categories

(DevTools :: Debugger, defect, P3)

15 Branch
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ioana_damy, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [testday-20120706])

Attachments

(1 file)

Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20120705 Firefox/15.0a2 (20120705042010)

STR:
1. Launch Firefox.
2. Load http://greensqr.com/misc/mozilla/debugger/ in the browser.
3. Go to main menu Tools->Web Developer and click on the Debugger.
4. Set a breakpoint at the 28th line (click on the left of the line number) and run the script.
5. Observe each line in the Stack viewer.

For each js files, the line displayed after ":" is correct. For HTMLs the same "1" is always displayed.
Whiteboard: [testday-20120706]
(In reply to Ioana Budnar [QA] from comment #0)
> For each js files, the line displayed after ":" is correct. For HTMLs the
> same "1" is always displayed.

This is because SpiderMonkey returns an arguably bogus URL+line combination for event handlers defined on DOM elements. Jim, Jason, is there any plan to fix this?

A complex hack that I thought about would require the DOM element association from bug 637572, and then have the debugger do some HTML parsing to find the event handler definition in the markup.
Priority: -- → P3
Summary: The first line is always referenced in the stack trace for HTMLs → The first line is always referenced in the stack trace for HTML / XUL documents
Currently we only get Debugger.Script objects for contents in <script> tags. If we received them for nodes with event handlers as well, we'd appear to return to the correct node in the HTML / XUL document.

This might give us an entry into debugging event handlers as well.
We'll probably be using bug 779306 for debugging event handlers.
So the problem with the event handler script in this test page is that Debugger.Script has the right URL (https://bug771490.bugzilla.mozilla.org/attachment.cgi?id=682477), but a startLine of 1 and a lineCount of 1.
This issue seems related to bug 699179 and perhaps the right long-term fix is bug 637572.
(Sorry, wasn't going to CC anyone)
This bug needs a specific fix that doesn't seem related to either bug mentioned in comment 6.

nsEventListenerManager::CompileEventHandlerInternal (in content/events/src/nsEventListenerManager.cpp) always passes a line number of either 0 or 1 to nsJSContext::CompileEventHandler.

Note that in order to obtain the scrap of JS code to compile, this method literally calls content->GetAttr(), passing the attribute name "onclick". Presumably the line number has been thrown away by this point; we'll have to find a way to store it.
In this particular case, the content comes from an nsHtml5TreeOperation with opcode eTreeOpCreateElementNetwork (frame #9 below). So we'd have to get a line number there, at least, from the HTML parser, then pipe it all the way up this stack:

#0  nsEventListenerManager::SetEventHandlerInternal()
         at nsEventListenerManager.cpp:563
#1   in nsEventListenerManager::SetEventHandler() at nsEventListenerManager.cpp:683
#2   in mozilla::dom::Element::SetEventHandler() at Element.cpp:1657
#3   in nsGenericHTMLElement::AfterSetAttr() at nsGenericHTMLElement.cpp:1715
#4   in nsGenericHTMLFormElement::AfterSetAttr() at nsGenericHTMLElement.cpp:3182
#5   in nsHTMLButtonElement::AfterSetAttr() at nsHTMLButtonElement.cpp:569
#6   in mozilla::dom::Element::SetAttrAndNotify() at Element.cpp:1864
#7   in mozilla::dom::Element::SetAttr() at Element.cpp:1762
#8   in nsGenericHTMLElement::SetAttr() at nsGenericHTMLElement.cpp:1808
#9   in nsHtml5TreeOperation::Perform() at nsHtml5TreeOperation.cpp:431
#10  in nsHtml5TreeOpExecutor::RunFlushLoop() at nsHtml5TreeOpExecutor.cpp:565
#11  in nsHtml5ExecutorReflusher::Run() at nsHtml5TreeOpExecutor.cpp:67

hsivonen, is this a reasonable thing to do?
(In reply to Jason Orendorff [:jorendorff] from comment #9)
> In this particular case, the content comes from an nsHtml5TreeOperation with
> opcode eTreeOpCreateElementNetwork (frame #9 below). So we'd have to get a
> line number there, at least, from the HTML parser, then pipe it all the way
> up this stack:
...
> hsivonen, is this a reasonable thing to do?

Yes. The only downside would be that the size of all tree ops would grow. The eTreeOpCreateElement* ops already use all 4 data slots on nsHtml5TreeOperation, so you'd need to add a 5th slot for the line number. (Or add another tree op type that transfers the line number, but that would be pointlessly complex.)

Do we want to have the line number available only for event handler compilation or do we want to make the DOM nodes themselves larger and store the line number for each node for other kinds of reporting purposes, too? (This would avoid propagating the line number up the stack mentioned in the previous comment as an argument.)
Another possibility would be to store on the attribute, instead of a string, an object that retains the line, column, URL, and code. That way we'd only spend memory when actually storing an event handler. And perhaps the extant piping would do most of the job for us.

I have no idea how disruptive it would be for that value to be an object instead of a string, though.
Jim, do you mind checking if this old bug still exists? I'm trying to clean up our backlog a bit and looking at very old bugs.
Flags: needinfo?(jimb)
Product: Firefox → DevTools
Flags: needinfo?(jimb)

This all works now as far as I'm aware.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: