Closed Bug 76531 Opened 23 years ago Closed 23 years ago

view source uses hard-coded pixel font size

Categories

(Core :: DOM: HTML Parser, defect)

Product:
Core
Component:
DOM: HTML Parser
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.1

People

(Reporter: jruderman, Assigned: bzbarsky)

References

(
URL
)

Details

Attachments

(2 files)

patch to fix this font thing
677 bytes, patch
Details | Diff | Splinter Review
Patch to fix me being dumb
563 bytes, patch
Details | Diff | Splinter Review 
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.

http://lxr.mozilla.org/seamonkey/source/layout/html/document/src/viewsource.css 
line 33.
The previous style that was placed on the <pre> that surround the viewsource
content was this one:

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

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 ian@hixie.ch 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.
-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
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....

reviews?
Keywords: mozilla0.9.1, patch, review
OS: Windows 98 → All
Hardware: PC → All
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.
r=rbs for attachment 31484 [details] [diff] [review]
Target Milestone: --- → mozilla0.9.1
Marc, can you sr this one?
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.
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.
Checked in, marking fixed.
Status: NEW → RESOLVED
Closed: 23 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.  :(
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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'?
 }
I restored the font-family to '-moz-fixed' as it was previously.
Re-resolving as FIXED.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Marking fixed as per the above developer comments.
Status: RESOLVED → VERIFIED
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?"
-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.

Attachment

General

Creator:
Created:
Updated:
Size: