Closed
Bug 226871
Opened 22 years ago
Closed 22 years ago
text in <tt> .. </tt> uses proportional font
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: rick, Assigned: dbaron)
References
()
Details
(Whiteboard: [patch])
Attachments
(2 files, 2 obsolete files)
|
402 bytes,
text/html
|
Details | |
|
13.81 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
brendan
:
approval1.6b+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031121
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20031121
If the preferences->appearance->fonts option "Allow Documents to use other
fonts" is selected, and a proportional font is in use, then text between <tt>
and </tt> appears in a proportional font, even though a suitable fixed font is
selected under preferences->appearance->fonts->monospace. This can destroy
formatting which relies on character width, the reason for including <tt>.
Reproducible: Always
Steps to Reproduce:
1.
2.
3.
|
||
Comment 1•22 years ago
|
||
WFM in 1.5 W98 and trunk OS/2.
|
||
Comment 2•22 years ago
|
||
WFM in W2k/1.6a, though I don't understand what 'when a proportional font is in
use' means. See attachment 10819 [details] for a simple testcase.
|
Reporter | |
Comment 3•22 years ago
|
||
Sorry, I can't follow the links to your test case. But here's mine, which I
should not have been so lazy as to omit the first time. Using the source:
<html>
<head>
<title>TT bug test</title>
</head>
<body>
<font face="Verdana,Helvetica,Arial,sans-serif">
Within each packet, data are represented as an ordered sequence of
10-bit unsigned binary numbers in the range 0-1000 inclusive. The
pairs are packed as sequential bits without regard to byte boundaries,
so that the packing pattern repeats after each two data pairs:
<pre><tt>
Content: | Phase 1 | Quad 1 | Phase 2 | Quad 2 | ...
Data bit: 98765432 10987654 32109876 54321098 76543210 ...
Byte: 76543210 76543210 76543210 76543210 76543210 ...
Transmit order: First-------------------------------------->last
</pre></tt>
</body>
</html>
With preferences->appearance->fonts option "Allow Documents to use other
fonts" selected I display the entire document in proportional font. With this
option unselected, or with the <font...> line omitted, the display correctly
shows the part within <tt> ... </tt> in a non-proportional font. The omission of
the <pre> ... </pre> tags makes no difference to the bug, but their inclusion
makes it a bit easier to see.
My system is Gentoo Linux, on a 900MHZ Athlon with 256M DDR SDRAM, compiled with
the vanilla kernel (i.e. Linux 2.4.22 kernel as found at www.kernel.org), rather
conservative optimisation, and not overclocked. Mozilla was compiled from source
on the same system, using gcc 3.2.3.
|
||
Comment 4•22 years ago
|
||
your tags are invalidly nested:
<font><pre><tt>text</pre></tt></font>
Mozilla's content model for that is:
<font></font>
<pre><font><tt>text</tt></font></pre>
<font></font>
so the problem is that:
<pre><font><tt>text</tt></font></pre>
gives proportional font (which is the second half of this testcase)
leaving out the <pre> in the testcase (as well as your original html) fixes the
bug.
|
|
||
Comment 5•22 years ago
|
||
confirmed with linux CVS trunk 20031126
==> stylesystem
Assignee: general → dbaron
Status: UNCONFIRMED → NEW
Component: Browser-General → Style System (CSS)
Ever confirmed: true
QA Contact: general → ian
|
|
Reporter | |
Comment 6•22 years ago
|
||
I'm not sure this is the whole story.
If I have to write <tt><font...> text </font></tt> there is no point in the <tt>
tags at all, since the <font> tags do all the work. The idea of <tt> is to force
a monosopaced font, temporarily, the effect vanishing after the </tt>.
As far as I can see, <font><pre><tt>text</tt></pre></font> is valid HTML.
|
||
Comment 7•22 years ago
|
||
Attachment #136437 -
Attachment is obsolete: true
|
|
||
Comment 8•22 years ago
|
||
I _think_ the problem is that nsRuleNode::ComputeFontData has the following code:
1962 // Now compute our font struct
1963 if (generic == kGenericFont_NONE) {
1964 // continue the normal processing
1965 // our default font is the most recent generic font
1966 generic = parentFont->mFlags & NS_STYLE_FONT_FACE_MASK;
....
1972 }
...
1979 // Set our generic font's bit to inform our descendants
1980 font->mFlags &= ~NS_STYLE_FONT_FACE_MASK;
1981 font->mFlags |= generic;
In other words, once you have a node that has a generic (and _only_ a generic,
which is a bug in itself) as its computed font-family, all descendants of it
will have the bit for that generic set.
Now SetGenericFont walks up the tree to the nearest ancestor with that generic
bit set and starts applying style rules from there. In this case, it will walk
up exactly one level, and start applying rules starting with that inline
font-face rule (or the "face" attribute on <font> in the original testcase).
The reason the original testcase shows the bug, of course, is that both <tt> and
<pre> have the same generic font applied to them in html.css.
Why exactly are we propagating generic bits down to kids like that? Shouldn't
we only or "generic" into mFlags if it wasn't NONE to start with?
|
|
||
Comment 9•22 years ago
|
||
Totally untested (will have to wait till my builds are done)
|
Assignee | |
Comment 10•22 years ago
|
||
I already discovered the same thing and was just compiling a patch that had that
plus a lot of cleanup in it.
|
|
Assignee | |
Comment 11•22 years ago
|
||
(Well, you probably found it first, but I wasn't reading my bugmail while I was
looking at the code. :-)
|
|
Assignee | |
Comment 12•22 years ago
|
||
This patch contains a lot of cleanup of the code, since I had to read it to
find the bug, and I couldn't resist. Most of it is obvious. I removed
nsRuleNode::GetBits and changed its one caller (and related function) to be
static methods on nsRuleNode so they have direct access to mNoneBits. The rest
should be obvious once you note that the two bits related to the
|higherContext| variable are not independent.
And I've tested that it works.
|
|
Assignee | |
Updated•22 years ago
|
Attachment #136649 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•22 years ago
|
Attachment #136650 -
Flags: superreview?(bz-vacation)
Attachment #136650 -
Flags: review?(bz-vacation)
|
|
Assignee | |
Updated•22 years ago
|
Whiteboard: [patch]
|
|
Assignee | |
Comment 13•22 years ago
|
||
Oh, and I'm removing
313 enum { // Types of RuleBits
314 eNoneBits,
315 eDependentBits
316 };
too, but I won't attach a new patch.
|
|
||
Comment 14•22 years ago
|
||
Comment on attachment 136650 [details] [diff] [review]
patch, with a lot of cleanup
Looks good. r+sr=bzbarsky
Attachment #136650 -
Flags: superreview?(bz-vacation)
Attachment #136650 -
Flags: superreview+
Attachment #136650 -
Flags: review?(bz-vacation)
Attachment #136650 -
Flags: review+
|
|
Assignee | |
Comment 15•22 years ago
|
||
Comment on attachment 136650 [details] [diff] [review]
patch, with a lot of cleanup
This should be a relatively low risk fix to a rather serious bug.
Attachment #136650 -
Flags: approval1.6b?
|
||
Comment 16•22 years ago
|
||
Comment on attachment 136650 [details] [diff] [review]
patch, with a lot of cleanup
I talked to dbaron about this last night, it's good for 1.6b.
/be
Attachment #136650 -
Flags: approval1.6b? → approval1.6b+
|
|
Assignee | |
Comment 17•22 years ago
|
||
Fix checked in to trunk, 2003-12-02 14:24 -0800.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•