The status line is shown by view source by correction of bug15364 at mozilla. I want the same function also at Firefox. There is a problem of bug 194380 in present mozilla view sourcede. When adding a function by Firefox, cautions are required also for this problem. Windows XP Mozilla/5.0 (Windows; U; Windows NT 5.1; ja-JP; rv:1.7) Gecko/20040613 Firefox/0.8.0+

Firstly, the status bar is very useful, mainly for displaying the Column number. Go to Line, while useful functionality, is not as useful as just scrolling through the file with line numbers displayed in parallel. When tracking down validation errors oftentimes all the context you have to go on is the line# and column# the validator gives back. If your site uses includes or any sort of programming, these numbers will not correspond to the numbers in your own local source file, so you need to be able to find the line in the online source (do a view source on the generated page) and then find the corresponding text in the local copy. The Mozilla suite's implementation of this is not as good as it could be. There should be a column running down the left side of the window listing the line numbers. This makes word-wrap line breaks obvious. I'm attaching an image of how this is represented in Homesite 4.5. The column can either be fixed width with a gutter as above, or can be sized dynamically based on the length of the source file. I'm not a Mozilla hacker, but to get the required width of the line# column you can get the number of lines of source code, which will be, say, 350. Then you know it needs to be 3 digits wide.

This looks like a DUPE of bug 207656

That bug looks like something to put the current line number in the status bar. That is a useless feature when looking for a particular line.

Having current cursor position (column and line) is very usefull when validating an html page !

So when a validator says there is an error on line 394, how fast can you find that by scrolling, clicking, and looking at the status bar? How many times do you have to do that to find the right line?

This is an extension by Chris Neale (http://cdn.mozdev.org/) that shows the line and column number of where the cursor is in the view source window in the status bar of the window and also adds the edit->goto line menu item that lets you jump to a certain line number directly. Maybe it is helpful for those that are desperate, want to improve on it or want to use it as a basis for a patch for this bug.

All that is really needed is something that says something like: foreach(line) { $i++ print i . ". $"; } Forgive the pseudo-Perl ... it's my native tongue.

I agree with comment 5; displaying the character in the status bar is not the ultimate solution (and it's coverted by bug 207656 anyway). I think the solution is to have line numbers to display in the left margin and ensure that copy-pasting the source won't also copy-paste those numbers. To do this, I believe the following would work: a) Change the code here http://lxr.mozilla.org/seamonkey/source/parser/htmlparser/src/nsViewSourceHTML.cpp#1194 to output a new pre element at every line break in the source (I don't know C++) b) Put in a CSS counter to show a number at the start of every pre element.

I would suggest something different (additional) to what others are saying. I wound really like to have an option in the View menu (checkbox) to Show Line Numbers. If checked, this would add a vertical bar along the left side of the screen with line numbers in it. Similar to VC or pretty much any other decent text/code editor. Of course, a Goto Line... feature would also be great.

here here - line numbers next to the lines !

This bug is absolutely critical to resolve. Firefox view source is like a complete joke without view line numbers.

I am using the excellent HTML Validator plugin to view HTML errors and warnings, and my biggest complaint is the lack of line numbers. The HTML Validator plugin author says that this is something mozilla/Firefox core needs to fix.

I know this bug has been neglected, but does anyone know anyone who can either A) fix this or B) mark it as a 2.0-blocker?

Can only nominate.

We're not blocking the release of Firefox 2 for this. It sounds like a worthwhile addition, though.

Yes, would be great to have it in :)

The newest beta of the HTML Validator plugin has support for line numbers in view source, although it's a little buggy at the moment. http://users.skynet.be/mgueury/mozilla/

Is this a dupe of bug 239221? (Or maybe dupe the other once since the last comment was at the end of 2004?) I'm not sure what the original reporter is talking about but it looks like there is a status bar for view source for the trunk build. As for implementing line numbers in the left column, it looks likes nsViewSourceHTML.cpp already adds a <pre id="line%"> tag at intervals defined by NS_VIEWSOURCE_TOKENS_PER_BLOCK in order to fix bug 86355. Can't I just output the tag for every line instead of intervals and just add a CSS before counter the pre tag at viewsource.css? (As suggested in a previous comment) I can then add a View -> Line Numbers menu option to enable or disable the css counter similar to the wrap lines implemention. It would also simply the go to line feature without all the treewalker business. Any reasons why the pre tag can't be outputted for every line?

Sorry, I meant bug 239211 not 239221.

No, bug 239211 is the SeaMonkey equivalent of this bug.

Why is this still not implemented? We are now at 3.1!

It would be really useful for any web developer to have ability to easily determine what exact line is line that Error Console referring to. "Go to Line" feature is not the same. Very not the same.

5 1/2 years? Is it sooo hard to fix?

Just notice for developers: even IE8 already shows line numbers in View Source.

This does the donkey work of actually adding numbers into the HTML (each line number in a <span class="linenumber">). There's still some issues with how best to use the span - the current CSS is a bit of a hack, but it works. (This patch also fixes an earlier mistake where I assigned a PRInt32 to a PRBool. Oops.)

(In reply to comment #28) > Created an attachment (id=432985) [details] > patch v1 > > This does the donkey work of actually adding numbers into the HTML (each line > number in a <span class="linenumber">). There's still some issues with how best > to use the span - the current CSS is a bit of a hack, but it works. > > (This patch also fixes an earlier mistake where I assigned a PRInt32 to a > PRBool. Oops.) Note that you can use CSS counters to generate the linenumbers for you, for example: body { counter-reset: line; } .linenumber:before { content: counter(line) ": "; counter-increment: line; } I haven't looked at the C++ code in detail yet, but I see that with your patch there is both a "mLineNumber" and a "myLineNumber". (Also, mrbkap's feedback is likely to be more useful than mine since he can actually approve your patch for landing.)

I *think* the only remaining piece is to make the space for line numbers smaller or larger as appropriate. Is there a way to know the total number of lines when I'm in BuildModel (ie. before we go through and count them all, so I can set an attribute on the body tag)?

(In reply to comment #30) > Created an attachment (id=440990) [details] > patch v2 > > I *think* the only remaining piece is to make the space for line numbers > smaller or larger as appropriate. Is there a way to know the total number of > lines when I'm in BuildModel (ie. before we go through and count them all, so I > can set an attribute on the body tag)? I personally think just assigning a fixed width like you're currently doing is fine.

Geoff: I discovered a bug with how the line numbers are displayed in the presence of a multi-line HTML comment. This is with the v2 patch.

Good point. And fixed.

I didn't look at the code too carefully yet (though there are various formatting and string usage issues there as far as I can tell). General questions: 1) Have you measured the memory consumption of view-source on large files with and without this patch? How does this patch affect it? 2) Have you measured the performance of view-source on large files with and without this patch? How does this patch affect it?

Comment on attachment 441458 [details] [diff] [review] patch v3 pending answers to my questions.

I don't really have the right tools to test these sorts of things (and I don't have jemalloc), but memory usage seems to be up about 10-15%. StartNewPreBlock seems to use a lot of time. Called every 40 lines the overall time was up 50-90% on some pages. :( If it's not called at all the overall time is up less than 10%.

(In reply to comment #36) > I don't really have the right tools to test these sorts of things (and I don't > have jemalloc), but memory usage seems to be up about 10-15%. You might look at a simple count of elements in the view source document, both with line numbers and without. E.g. "javascript:alert(document.body.getElementsByTagName("*").length)". As I recall, the performance difference I saw for view-source linkification pretty closely correlated to the number of elements created. > StartNewPreBlock seems to use a lot of time. Called every 40 lines the overall > time was up 50-90% on some pages. :( If it's not called at all the overall time > is up less than 10%. That seems surprising. (Alas, I have no other insight to offer.)

Fixes multiple-line tags in error and a few other bits and pieces.

This is a dirt-simple mochitest for view-source. It verifies that the source displayed by view-source actually matches the true source. It doesn't test formatting issues or the display of line numbers in the main patch. Notably it won't detect the multi-line comment bug in the v3 patch. It does provide some confidence that changes don't break existing view-source functionality. Right now the test uses itself as the test source. It would benefit from better test source files. Note that the test file can be run directly in the browser with minor modifications (e.g. firefox file://.../view-source.html).

(In reply to comment #39) > Created an attachment (id=448324) [details] > Simple view-source test Very nice. FYI the URL appearing at the beginning is due to the title tag in the view-source page. >+nsresult CViewSourceHTML::StartLine(void){ >+ //if (mLineNumber % 40 == 0) { >+ // StartNewPreBlock(); >+ //} I think it best if StartNewPreBlock is called every 50 lines, it seems to be a good tradeoff between initial speed and UI responsiveness. I also think that the line numbers column can be narrowed to 4 characters instead of 5. It's highly unlikely that people would use view source for a 10,000 line page.

(In reply to comment #40) > I also think that the line numbers column can be narrowed to 4 characters > instead of 5. It's highly unlikely that people would use view source for a > 10,000 line page. Actually I think this is moderately common. I know a lot of work went in to Firebug to handle long files. On the other hand, I'm not sure that we need to look pretty when handling really long files. For example, if the main content gets bumped over by an extra character going from line 9999 to 10000, that may not be such a big deal.

Jeff, it's likely to be at least a week (two is more likely) until I get to this review...

(In reply to comment #42) > Jeff, it's likely to be at least a week (two is more likely) until I get to > this review... Is two weeks up yet Boris? :)

Yes, but people keep dumping emergencies on me. :( And my sincere apologies for mis-spelling your name.... Current time estimate is ... sometime. If no one else dumps 1.5MB "must review now" crap on me, should be within a week.

Comment on attachment 446641 [details] [diff] [review] patch v4 (same as bug 536503 - can't ask for approval until review is completed)

I've added these tests to make sure the patch does as it's supposed to. Kept as a separate patch for clarity's sake.

Ok. So I really suck at getting to this review, mostly because I keep not finding the time to page this code back in. :( Geoff, I'm really sorry about that, and about my complete lack of communication here. And now Henri is in the process of removing this whole file and rewriting view-source on top of the HTML5 parser... Henri, would you be willing to effectively roll these patches into your rewrite or do something like this as a followup or something?

(In reply to comment #48) > Henri, would you be willing to effectively roll these patches into your rewrite > or do something like this as a followup or something? As a follow-up would work. The rewrite is now big enough that I have worries about it getting it reviewed and landed, so I don't want new features added before the rewrite has landed. It seems to me that the patch here regresses bug 86355. Am I missing something?

The patch still does the multiple <pre> block thing. It also puts each line into a separate block, which breaks bidi stuff even more than bug 86355 did...

Thanks for the update Boris. I'll have another look at this once the rewrite is done.

Comment on attachment 448324 [details] [diff] [review] Simple view-source test r=me

Can the reviewed test be landed?

This is effectively the same CSS as the previously reviewed patch, the other changes are unnecessary now.

I've made a few slight changes to Curtis's test.

Comment on attachment 571213 [details] [diff] [review] fixed-up test Drive-by comment: >+ return xhr.responseText; If you are only reading responseText, please set xhr.responseType = "text" to make the test run faster.

Comment on attachment 571213 [details] [diff] [review] fixed-up test The test has two problems: 1. It doesn't use SimpleTest.waitForExplicitFinish/finish, so it effectively doesn't test anything. 2. It loads the same test in an iframe, which could potentially cause _another_ instance of the test to run. I suggest that you add a test HTML file and test the view-source protocol on that URL instead. Also, please address comment 56 in the next version of the test. Thanks!

(In reply to Ehsan Akhgari [:ehsan] from comment #57) > 2. It loads the same test in an iframe, which could potentially cause > _another_ instance of the test to run. I suggest that you add a test HTML > file and test the view-source protocol on that URL instead. It's loading the source of the test in the iframe, not the test itself.

Comment on attachment 577870 [details] [diff] [review] tests Review of attachment 577870 [details] [diff] [review]: ----------------------------------------------------------------- Looks great, thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/4730eb3ec77a https://hg.mozilla.org/integration/mozilla-inbound/rev/389a18921ea5 Okay, I've pushed this. It doesn't currently work for non-HTML documents, so I'll file a followup and hopefully have it fixed by the end of this cycle.

Backed out, reftest bustage. :( https://hg.mozilla.org/integration/mozilla-inbound/rev/9f29daaecbcc

Looks like the reference images need to be updated to include the line numbers.

Comment on attachment 577870 [details] [diff] [review] tests https://hg.mozilla.org/mozilla-central/rev/389a18921ea5

Comment on attachment 584319 [details] [diff] [review] fix for broken reftests Please add a comment why empty id attributes. And, if possible, remove =""

Take 2 https://hg.mozilla.org/integration/mozilla-inbound/rev/02b3a12aea70 https://hg.mozilla.org/integration/mozilla-inbound/rev/39d88d1cd7d0

https://hg.mozilla.org/mozilla-central/rev/02b3a12aea70 https://hg.mozilla.org/mozilla-central/rev/39d88d1cd7d0