view source uses hard-coded pixel font size

VERIFIED FIXED in mozilla0.9.1



HTML: Parser
17 years ago
17 years ago


(Reporter: Jesse Ruderman, Assigned: bz)



Firefox Tracking Flags

(Not tracked)




(2 attachments)



17 years ago
View source now fixes the font size at 12px.  It used to honor the monospace 
font setting from prefs, which is 13px (courier) by default.  I think this 
changed when bug 74486 was fixed. 
line 33.

Comment 1

17 years ago
The previous style that was placed on the <pre> that surround the viewsource
content was this one:

static const char* kPreStyle = "font-family: -moz-fixed; font-weight:normal; 
color:black; padding-top:4px; margin-left:4px;";

The current one is the one that was sitting in the existing viewsource.css
(which used to be used for the other viewsource in XML).

So you are suggesting to reinstante what was in kPreStyle, i.e:,
.viewsource { 
- font-family: monospace;
+ font-family: -moz-fixed;
  font-weight: normal; 
  color: black; 
- white-space: pre;
- font-size: 12px; 
- padding-top: 8px; 
- margin-left: 8px;
  padding-top: 4px; 
  margin-left: 4px;

Adding to cc:list for any comments.
I would say that getting rid of 12px size is a good idea... and -moz-fixed seems
to be a slightly different size (maybe also different font?) from monospace, btw.

Comment 3

17 years ago
-moz-fixed  means the user' selected fixed-size font in the pref dialog.
monospace means any fixed-size font that could be fetched from the OS.
-moz-fixed is what is needed because it would then be configurable (until
that other GUI bug -- if it ever gets implemented...)
rbs, good point.  Patch coming up
Created attachment 31484 [details] [diff] [review]
patch to fix this font thing
OK.  use -moz-fixed for the font-family

removing the random top and left space -- the space given by the margins on
<html> is plenty....

Keywords: mozilla0.9.1, patch, review
OS: Windows 98 → All
Hardware: PC → All

Comment 7

17 years ago
Does "font-size: normal" cause the user' pref' size to be used? Or is that
override the pref' value?
That causes the user pref size to be used.

Comment 9

17 years ago
r=rbs for attachment 31484 [details] [diff] [review]
Target Milestone: --- → mozilla0.9.1
Marc, can you sr this one?

Comment 11

17 years ago
Change looks good, Boris. sr=attinasi

Why did you remove the padding though? In general, you should consider using the
'em' unit instead of pixels - it scales with the font.
I removed the padding because .viewsource is the outermost container.  And we
already have margins on <html> (from html.css) giving us some space between the
content and the edge of the window....

The extra padding was just superfluous.

Comment 13

17 years ago
I agree with removing the extra padding.  The extra padding was making it look 
like there was a blank line at the top of the file and a space at the beginning 
of each line.

Comment 14

17 years ago
Checked in, marking fixed.
Last Resolved: 17 years ago
Resolution: --- → FIXED
Ugh.  I'm gonna reopen this one...  I was stupid and regressed it (partially)
with the patch to bug 62678...

For some reason it looks like we can't use

font-family: -moz-fixed, monospace;

The -moz-fixed gets ignored.  This is no good.

Patch coming up.  Reviews please?  And my apologies for all the spam.  :(
Resolution: FIXED → ---
Created attachment 32732 [details] [diff] [review]
Patch to fix me being dumb

Comment 17

17 years ago
Marc, knowing that viewsource is really <pre class="viewsource">...</pre>, what
will happen if the 'font-family: -moz-fixed' and the 'whitespace: pre' are
just remove from the viewsource style sheet?

 .viewsource {
-  font-family: -moz-fixed, monospace;
   font-weight: normal;
   font-size: normal;
   color: black;
-  white-space: pre; // is this 'white-space' or 'whitespace'?

Comment 18

17 years ago
I restored the font-family to '-moz-fixed' as it was previously.
Re-resolving as FIXED.
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 19

17 years ago
Marking fixed as per the above developer comments.

Comment 20

17 years ago
bzbarsky, remember your patch that caused a regression here?
-  font-family: -moz-fixed;
+  font-family: -moz-fixed, monospace;

I just saw that it was because the special handling of -moz-fixed in the 
style system is based on the _equality_ of the font-family with -moz-fixed.
i.e., -moz-fixed should always be the only font...
Ugh.  "Who ordered that?"

Comment 22

17 years ago
-moz-fixed is special, by definition. So it is treated in a peculiar manner.
(See the code in content/html/style/src/nsRuleNode.cpp if you are interested.)
You need to log in before you can comment on or make changes to this bug.