Variable tabulator width instead of fixed width of 4 spaces

RESOLVED FIXED in mozilla1.9.3a1

Status

()

enhancement
RESOLVED FIXED
14 years ago
10 years ago

People

(Reporter: julius, Assigned: darktrojan)

Tracking

Trunk
mozilla1.9.3a1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Reporter

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

It would be nice to be able to use variable tabulator widths instead of the
fixed width of 4 spaces. Personally I work with a tabulator width of 2 spaces in
my editor and when I view the code of a website with firefox it's very
irritating to have a tabulator width of 4 spaces.

Reproducible: Always

Comment 1

14 years ago
confirming as a valid enhancement request, but I don't think this will be
implemented, as Firefox's view source is intended to be a very simple source viewer.
URL: http
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
Version: unspecified → Trunk
Duplicate of this bug: 390935

Comment 3

12 years ago
As I mentioned in bug #390935 (duplicate of this bug now) I consistently see a tab width of 8 characters, not 4 (my preference is 2).  A tab width of 8 characters can move some deeply indented lines halfway across the view source window.

A preference in about:config to set the view source tab width would be great!
Product: Firefox → Toolkit
Assignee

Comment 4

10 years ago
A small amount of investigation reveals:

* 8 space widths is hard-coded into ComputeTabWidthAppUnits in layout/generic/nsTextFrameThebes.cpp
* This is because the CSS specifies tabs preformatted text must be equal to 8 spaces in width

Can't see either of these changing soon, short-sighted as they seem.

Comment 5

10 years ago
Thanks, Geoff, for the explanation of where the problem lives.

I, too, have long despised the eight space tab.  Surely the rules of css do not apply to an application displaying source material.  I can see that the source viewer might default to an existing stylesheet, but would it be all that difficult to create a viewer-only stylesheet, and have the viewer link to it?

I apologize if I'm talking through my hat; my C coding experience is so close to zero as to be indistinguishable.
Assignee

Comment 6

10 years ago
Posted patch tab widths as a pref (obsolete) — Splinter Review
This is probably going to break a million tests, cause a performance hit, and generally annoy a few things. Especially as it defies the CSS specification.

But it does work.
Assignee

Comment 7

10 years ago
Gary: The major problem is that the view-source page is actually an HTML page, and is displayed using all the same code as a normal page.

Robert, I don't know if you're the right person to talk to about this, but I have some questions (mostly because I'm delving into code I don't know and don't really understand):

Is getting this pref here a bad idea? How often is ComputeTabWidthAppUnits called? Should I cache the answer somewhere?

What tests is this going to break? The only tests I've worked with before are XPCShell tests and I don't think they're really going to help here.
Component: View Source → Layout: Text
Product: Toolkit → Core
QA Contact: view.source → layout.fonts-and-text
The best approach here would be to implement -moz-tab-size similar to Opera's -o-tab-size. See
http://lists.w3.org/Archives/Public/www-style/2008Dec/0009.html

It would be pretty easy to do. You'd add it to nsStyleText.
Assignee

Updated

10 years ago
Depends on: 517882
Assignee

Updated

10 years ago
Attachment #401789 - Attachment is obsolete: true
Assignee

Comment 9

10 years ago
Posted patch patch v1 (obsolete) — Splinter Review
This patch sets -moz-tab-size using the style attribute on the BODY node in a view-source page.

I'm considering adding UI to the view source window, but for now it just works off a pref.
Assignee: nobody → geoff
Status: NEW → ASSIGNED
Assignee

Comment 10

10 years ago
Posted patch patch v2 (obsolete) — Splinter Review
Patch including front-end for view source window
Attachment #401980 - Attachment is obsolete: true
Attachment #402269 - Flags: review?(gavin.sharp)
Assignee

Updated

10 years ago
Component: Layout: Text → View Source
Product: Core → Toolkit
QA Contact: layout.fonts-and-text → view.source

Updated

10 years ago
Duplicate of this bug: 521419

Comment 12

10 years ago
a setting for the standard tab-size would be also great. (maybe in the window where you can set your standard sans, serif and monospace font):
a small field where you can enter a digit from 2 to 9 (everything else is useless)

Comment 13

10 years ago
oh, sorry: i want the setting to be used for the whole firefox. (e.g. viewing source code on web pages)
maybe my bug is no duplicate, then?

is the -moz-tab-size attribute also available for normal use? (so you can just use “* {-moz-tab-size:4}” in the userstyle.css or as a stylish-style)
Duplicate of this bug: 433543
Comment on attachment 402269 [details] [diff] [review]
patch v2

I think UI for this is probably overkill. Let's just add the viewsource backend changes so that people who want this can just change the pref manually?

I think mrbkap is the best person to review those changes.
Attachment #402269 - Flags: review?(gavin.sharp) → review-
Assignee

Comment 16

10 years ago
Posted patch patch v3 (obsolete) — Splinter Review
Attachment #402269 - Attachment is obsolete: true
Attachment #424330 - Flags: review?(mrbkap)
Comment on attachment 424330 [details] [diff] [review]
patch v3

>@@ -397,16 +402,23 @@ NS_IMETHODIMP CViewSourceHTML::BuildMode
>                         NS_LITERAL_STRING("id"),
>                         NS_ConvertASCIItoUTF16(kBodyId));
> 
>           if (mWrapLongLines) {
>             AddAttrToNode(bodyNode, theAllocator,
>                           NS_LITERAL_STRING("class"),
>                           NS_ConvertASCIItoUTF16(kBodyClassWrap));
>           }
>+          if (mTabSize >= 0) {
>+            nsAutoString styleValue = NS_ConvertASCIItoUTF16(kBodyTabSize);
>+            styleValue.AppendInt (mTabSize);

Nit: No space before (.

>diff --git a/parser/htmlparser/src/nsViewSourceHTML.h b/parser/htmlparser/src/nsViewSourceHTML.h
>     PRPackedBool        mSyntaxHighlight;
>     PRPackedBool        mWrapLongLines;
>+    PRInt32             mTabSize;
>     PRPackedBool        mHasOpenRoot;
>     PRPackedBool        mHasOpenBody;

Please move mTabSize out of the PRPackedBools so that they pack correctly (before mSyntaxHighlight should work).

Looks good otherwise.
Attachment #424330 - Flags: review?(mrbkap) → review+
Assignee

Comment 18

10 years ago
Posted patch patch v4Splinter Review
Right, let's get this fixed at long last!
Attachment #424330 - Attachment is obsolete: true
Assignee

Updated

10 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/a7e6a9d1b8ea
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Assignee

Comment 20

10 years ago
For userdocs: the preference "view_source.tab_size" sets the width of a tab character in the view source window. Already open windows aren't updated.
Keywords: user-doc-needed
Removing user-doc-needed.
We don't document anything that encourages the user to go to about:config, unless it is for troubleshooting or it is a frequently asked question.
Keywords: user-doc-needed
Historically, all preferences are documented on kb.mozillazine.org: I added a mention of the new pref to http://kb.mozillazine.org/About:config_entries#View_source.
Assignee

Comment 23

10 years ago
(In reply to comment #21)
> Removing user-doc-needed.
> We don't document anything that encourages the user to go to about:config,
> unless it is for troubleshooting or it is a frequently asked question.

(In reply to comment #22)
> Historically, all preferences are documented on kb.mozillazine.org: I added a
> mention of the new pref to
> http://kb.mozillazine.org/About:config_entries#View_source.

Okay, that's the outcome I wanted anyway.
You need to log in before you can comment on or make changes to this bug.