The HTML file attached to http://code.google.com/p/fbug/issues/detail?id=1628 has a top level script with a baseLineNumber of 7. However the source begins on line 29. The problem disappears if the tabs and blanks in the HTML file are removed. So somehow the line number counting mechanism in jsd is broken.

Actually this seems to come up more often than I thought.

This is an HTML parser bug, not a JS engine bug. /be

I suspect I might have to hide from Damon for a couple of days for even having come up with this patch, but I idly glanced at the testcase and the bug is obvious. It would be really good if someone could write more comprehensive (automated) testcases. jjb: if you care about this enough, please mark it wanted? with a strong case that we really care about this on 1.9.1. I suspect that, at this point, it's too late for this bug (at least for Firefox 3.5.0).

(In reply to comment #4) ... > jjb: if you care about this enough, please mark it wanted? with a strong case > that we really care about this on 1.9.1. I suspect that, at this point, it's > too late for this bug (at least for Firefox 3.5.0). Wow, this is great. I honestly never imagined this would be fixed. I'll start reporting more of these ;-) Regarding 3.5.0, this bug is annoying and makes our tools act unprofessional, but the workaround in this case is straight-forward. Many developers with auto-cleanup editors don't see it. Assuming the patch could be accepted for 3.5.1, I'll save my begging for bug 494235. Thanks!

"looking unprofessional" is a good reason to make this wanted. Thanks for the patch, mrbkap!

It would be great if this could be fixed as it causes us a huge amount of problems, often making scripts close to impossible to debug.

Does the attached patch work? Would be great to get Firebug people's feedback on a try-server build (robcee can kick one off).

Just to point out my mistake: The line number errors cause major havoc if you try either 'debugger' keyword or Break on Errors in Firebug. Firebug will either break somewhere bizarre or it will get confused because the break appears to happen off the end of the file. And of course what do users do if the lines appear to be off set for normal breakpoints? They try 'debugger' keyword and Break on Errors :-(.

Bug 495663 is another example of incorrect line numbers. That one depends on the presence of a <title/> element.

We need to know ASAP if this patch actually fixes the problem, and if it passes our tests. Please get it through the try server today if you want it in 3.5.

I opened https://build.mozilla.org/sendchange.cgi I checked " Use another Mercurial repository" and entered http://hg.mozilla.org/releases/mozilla-1.9.1 I downloaded and uploaded the patch. I left the other fields except description at their defaults since I don't know what to put in. After I hit "submit" I get a reply that the patch had been uploaded. But there was no information about a build being started. I clicked the link and got a tinderbox-like page, but I could not make sense of the result. I don't know if any build is running or were to go to get the result.

The result from comment 12 is at http://build.mozilla.org/tryserver-builds/johnjbarton@johnjbarton.com-1243781819 I installed the windows version. It was not a FF 3.5 build but a FF 3.6 build. I used about:config to add extensions.checkCompatibility false, so Firebug 1.4 would enable. I ran the test case: http://fbug.googlecode.com/issues/attachment?aid=-4759830275129926495&name=wrong_line_numbers_in_debugger.html from http://code.google.com/p/fbug/issues/detail?id=1628 with the script panel enabled and after loading the page. Firebug shows the correct line when it breaks on 'debugger'. I went back to Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1pre) Gecko/20090528 Shiretoko/3.5pre and repeated the same test. Firebug shows an incorrect line when it breaks.

I'm unable to tell if the patch was added to this build or if it's just because of additional JS fixes in mozilla-central that allowed this test to work. It was definitely a trunk build based on the log in: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1243782088.1243789406.1004.gz&fulltext=1 Also, the build showed 5 leak test errors. I can kick off another try build to see if this works against mozilla-1.9.1 but it's going to take a good few hours to complete.

according to Ben Hearsum, senchange.cgi does accept patches properly now. I'm going to do a try run with this patch against releases/1.9.1 and see how it goes. jjb: hopefully this works. I'm hoping you just missed the radiobutton on sendchange.cgi and kept the default mozilla-central repository.

verified that this builds and fixes the problem on Linux using: http://build.mozilla.org/tryserver-builds/rcampbell@mozilla.com-1243882150 any chance we can get this in?

still waiting on Try unittest and talos results, fyi.

results in from linux try unittest: Summary of unittest results: check: 6/0 xpcshell-tests: 550/0 reftest: 2650/0/150 crashtest: 1147/0/36 mochitest-plain: 77553/0/1563&nbsp;<em class="testfail">LEAK</em> mochitest-chrome: 1176/0/30 mochitest-browser-chrome: 2959/0/5 mochitest-a11y: 1194/0/78 we're seeing some leak failures same as for jjb's try on trunk. mrbkap: any chance you can take a look at the logs to see what's up? http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1243882237.1243887974.19624.gz

some reftest and browser-chrome failures on OS X. Summary of unittest results: check: 6/0 xpcshell-tests: 549/0 reftest: 2666/<em class="testfail">2</em>/132 crashtest: 1148/0/35 mochitest-plain: 77617/0/1568 mochitest-chrome: 1743/0/30 mochitest-browser-chrome: 2932/<em class="testfail">1</em>/5 http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1243882237.1243889449.22169.gz

I don't see us leaking any tokens there, which is literally the only thing this patch could have caused leaks of, so I'd say those are random leaks that we can safely ignore.

ok, I thought that might be the case but wanted to make sure before I made a grand pronouncement. :) Mac rendering errors could be spurious as well. Given that, can we can get some review-juice on this? lastly, no warnings on windows: Summary of unittest results: check: 31/0 xpcshell-tests: 550/0 reftest: 2659/0/141 crashtest: 1147/0/36 mochitest-plain: 77809/0/1562 mochitest-chrome: 1473/0/29 mochitest-browser-chrome: 2945/0/5 mochitest-a11y: 1194/0/78

Comment on attachment 380034 [details] [diff] [review] Mildly tested fix Tests please!

(In reply to comment #5) > (In reply to comment #4) > Regarding 3.5.0, this bug is annoying and makes our tools act unprofessional, > but the workaround in this case is straight-forward. Many developers with > auto-cleanup editors don't see it. We submitted the bug to firebug (and then John dropped the hot potato here to you Firefox crew), but this problem is also especially annoying when pages are generated by for example JSPs. And except writing a custom servlet filter to remove all those empty blanks and newlines after the page is generated (we did that for development purposes because of this issue), it rather makes firebug quite unusable in the mentioned cases. But great news a patch has been made!

yes. so close. is the above testcase adequate to prove this works? I tested with this patch and it fixes the problem, but would still love to get it checked in for 1.9.1. what about checking into m-c to bake for a few?

I'm confused by the info here. It looks to me like the manual tests show the fix does its job. The unit tests have some results which don't seem related to this patch, If the same tests are run on the same code base with out the patch, what is the result? I guess that sicking is asking for a automatic unit test that fails before the patch and succeeds with the patch, correct?

ok, I'm confused too. I thought this testcase failed on 1.9.1. It certainly behaves strangely. First load didn't position the script editor on the correct location. Using b4 seems to disable Firebug entirely on reload.

I think you are mixing this bug up with some other bug. This is a small patch unlikely to cause the issues you are seeing.

I don't understand comment 26 -- this bug is not patched in 1.9.1, so any testcase strange behavior there, or Firebug problems on beta 4, are another bug as JJB says. I read the patch, as a drive-by super-reviewer. It looks good. Let's take it. /be

Sorry for the confusion, I actually landed this last night, but had to leave before I got a chance to mark the bug: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/10490a7e8532 and http://hg.mozilla.org/mozilla-central/rev/2b079266b445

Verified fixed on trunk and 1.9.1 with the following builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090604 Minefield/3.6a1pre ID:20090604031228 Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre ID:20090604031153

A similar problem has been reported in Bug 504565, in case someone on this CC list has some insight.

clearing some ancient flags